* Re: [PATCH v4 2/6] mfd: bd71837: Devicetree bindings for ROHM BD71837 PMIC
From: Matti Vaittinen @ 2018-05-31 7:17 UTC (permalink / raw)
To: Rob Herring
Cc: Matti Vaittinen, mturquette, sboyd, mark.rutland, lee.jones,
lgirdwood, broonie, mazziesaccount, linux-clk, devicetree,
linux-kernel, mikko.mutanen, heikki.haikola
In-Reply-To: <20180531030129.GA16122@rob-hp-laptop>
Hello Rob,
Thanks for the review!
On Wed, May 30, 2018 at 10:01:29PM -0500, Rob Herring wrote:
> On Wed, May 30, 2018 at 11:42:03AM +0300, Matti Vaittinen wrote:
> > Document devicetree bindings for ROHM BD71837 PMIC MFD.
> > + - interrupts : The interrupt line the device is connected to.
> > + - interrupt-controller : Marks the device node as an interrupt controller.
>
> What sub blocks have interrupts?
The PMIC can generate interrupts from events which cause it to reset.
Eg, irq from watchdog line change, power button pushes, reset request
via register interface etc. I don't know any generic handling for these
interrupts. In "normal" use-case this PMIC is powering the processor
where driver is running and I do not see reasonable handling because
power-reset is going to follow the irq.
This IRQ might be relevant if use for PMIC is such that it is not
powering the processor where the driver is runninng. Then the controlling
processor can get the notification that chips powered by PMIC are
resetting. But handling for this must be use-case specific, right? So in
short - none of the current sub-devices use the IRQs - they are there
for specific use-cases which some boards may implement. Any suggestions
how to document the available interrupts? (power button line, sw reset
etc). My current assumption has been that one who is interested in using
these irqs should really also see the data-sheet for IRQs. But I admit
that documenting available interrupts here would be helpful. I will just
cook up some explanation and send it as a patch if no suggestions on how
to document those.
Patches 3/6 and 6/6 from the series were already applied to Mark's tree.
So how should I send further patches? Should I still send the whole series
(including already applied patches 3/6 and 6/6) or only the ones I change?
> > +Example:
> > +
> > + pmic: bd71837@4b {
>
> Node names should be generic ideally. So "pmic@4b"
I'll change that.
> > + clk: bd71837-32k-out {
>
> clock-controller {
And I'll change that too.
>
> > + compatible = "rohm,bd71837-clk";
> > + #clock-cells = <0>;
> > + clock-frequency = <32768>;
>
> Can this be anything else?
Not so that I know. Frequency is fixed. Is there a problem with this?
Br,
Matti Vaittinen
^ permalink raw reply
* Re: [PATCH v4 3/6] regulator: bd71837: Devicetree bindings for BD71837 regulators
From: Matti Vaittinen @ 2018-05-31 7:21 UTC (permalink / raw)
To: Rob Herring
Cc: Matti Vaittinen, mturquette, sboyd, mark.rutland, lee.jones,
lgirdwood, broonie, mazziesaccount, linux-clk, devicetree,
linux-kernel, mikko.mutanen, heikki.haikola
In-Reply-To: <20180531030436.GB16122@rob-hp-laptop>
On Wed, May 30, 2018 at 10:04:36PM -0500, Rob Herring wrote:
> On Wed, May 30, 2018 at 11:42:32AM +0300, Matti Vaittinen wrote:
> > Document devicetree bindings for ROHM BD71837 PMIC regulators.
> > +ROHM BD71837 Power Management Integrated Circuit (PMIC) regulator bindings
> > +
> > +BD71837MWV is a programmable Power Management
> > +IC (PMIC) for powering single-core, dual-core, and
> > +quad-core SoC’s such as NXP-i.MX 8M. It is optimized
> > +for low BOM cost and compact solution footprint. It
> > +integrates 8 Buck regulators and 7 LDO’s to provide all
> > +the power rails required by the SoC and the commonly
> > +used peripherals.
>
> Why duplicate this from the core binding?
I can remove this. I just thought it is nice to see what this chip is
doing even without opening the MFD binding doc. Just same question as in
the other patch - how should I deliver the change? This was already
applied to Mark's tree - should I do new patch on top of the Mark's tree
- or do patch against tree which does not yet contain this change? If I
do it on top of Mark's tree then Mark should apply it, right? If I do
it against some other three, thene there will be merge conflict with
Mark, right?
>
> Otherwise,
>
> Reviewed-by: Rob Herring <robh@kernel.org>
Thanks!
Br,
Matti Vaittinen
^ permalink raw reply
* Re: [PATCH v2 0/6] mfd/regulator/clk: bd71837: ROHM BD71837 PMIC driver
From: Matti Vaittinen @ 2018-05-31 7:26 UTC (permalink / raw)
To: Stephen Boyd
Cc: Lee Jones, Matti Vaittinen, Matti Vaittinen, mturquette, robh+dt,
mark.rutland, lgirdwood, broonie, linux-clk, devicetree,
linux-kernel, mikko.mutanen, heikki.haikola
In-Reply-To: <152769382760.144038.10609724773770264610@swboyd.mtv.corp.google.com>
On Wed, May 30, 2018 at 08:23:47AM -0700, Stephen Boyd wrote:
> Quoting Lee Jones (2018-05-30 04:16:49)
> > On Tue, 29 May 2018, Matti Vaittinen wrote:
> >
> > > Hello,
> > >
> > > On Tue, May 29, 2018 at 08:39:58AM +0100, Lee Jones wrote:
> > > > On Mon, 28 May 2018, Matti Vaittinen wrote:
> > > >
> > > > > Patch series adding support for ROHM BD71837 PMIC.
> > > > FYI, this patch-set is going to be difficult to manage since it was
> > > > not sent 'threaded'.
> > > >
> > > > If/when you send a subsequent version, could you please ensure you
> > > > send the set threaded so the patches keep in relation to one another
> > > > as they are reviewed?
> > >
> > > Thanks for the guidance. I have not sent so many patches to community so
> > > I am grateful also from all the practical tips =) Just one slight problem.
> > > I have only seen emails being threaded when one is replying to an email.
> > > So how should I send my patches in same thread? Just send first one and
> > > then send subsequent patches as replies?
> > >
> > > I just killed some unused definitions and one unused variable from the
> > > code so I am about to send new version. I'll try doing that as a threaded
> > > series and resend all the patches as v3.
> >
> > You don't need to do this manually.
> >
> > Just use `git send-email` with the correct arguments.
> >
>
> I usually send with 'git send-email *.patch' so that git can do the
> threading for me. Looks like these patches were sent with Mutt though,
> so perhaps 'git format-patch | git imap-send' was used without the
> --thread option on format-patch.
Yep. I used git format-patch without the --thread and mutt -H for
sending. Learned the --thread after Lee pointed out the threading and
used it for further series. Thanks for the help!
Br,
Matti Vaittinen
^ permalink raw reply
* Re: [PATCH] PCI: mediatek: Add system pm support for MT2712
From: Honghui Zhang @ 2018-05-31 7:37 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: lorenzo.pieralisi, marc.zyngier, bhelgaas, matthias.bgg,
linux-arm-kernel, linux-mediatek, linux-pci, linux-kernel,
devicetree, yingjoe.chen, eddie.huang, ryder.lee, youlin.pei,
hongkun.cao, sean.wang, xinping.qian, yt.shen, yong.wu
In-Reply-To: <20180531042020.GQ39853@bhelgaas-glaptop.roam.corp.google.com>
On Wed, 2018-05-30 at 23:20 -0500, Bjorn Helgaas wrote:
> On Wed, May 30, 2018 at 10:35:36AM +0800, honghui.zhang@mediatek.com wrote:
> > From: Honghui Zhang <honghui.zhang@mediatek.com>
> >
> > The MTCMOS of PCIe Host for MT2712 will be off when system suspend, and all
> > the internel control register will be reset after system resume. The PCIe
> > link should be re-established and the related control register values should
> > be re-set after system resume.
> >
> > Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> > CC: Ryder Lee <ryder.lee@mediatek.com>
> > ---
> > drivers/pci/host/pcie-mediatek.c | 82 ++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 82 insertions(+)
> >
> > diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
> > index dabf1086..60f98d92 100644
> > --- a/drivers/pci/host/pcie-mediatek.c
> > +++ b/drivers/pci/host/pcie-mediatek.c
> > @@ -132,12 +132,14 @@ struct mtk_pcie_port;
> > /**
> > * struct mtk_pcie_soc - differentiate between host generations
> > * @need_fix_class_id: whether this host's class ID needed to be fixed or not
> > + * @pm_support: whether the host's MTCMOS will be off when suspend
> > * @ops: pointer to configuration access functions
> > * @startup: pointer to controller setting functions
> > * @setup_irq: pointer to initialize IRQ functions
> > */
> > struct mtk_pcie_soc {
> > bool need_fix_class_id;
> > + bool pm_support;
> > struct pci_ops *ops;
> > int (*startup)(struct mtk_pcie_port *port);
> > int (*setup_irq)(struct mtk_pcie_port *port, struct device_node *node);
> > @@ -1179,12 +1181,91 @@ static int mtk_pcie_probe(struct platform_device *pdev)
> > return err;
> > }
> >
> > +static int __maybe_unused mtk_pcie_suspend_noirq(struct device *dev)
> > +{
> > + struct platform_device *pdev;
> > + struct mtk_pcie *pcie;
> > + struct mtk_pcie_port *port;
> > + const struct mtk_pcie_soc *soc;
> > +
> > + pdev = to_platform_device(dev);
> > + pcie = platform_get_drvdata(pdev);
> > + soc = pcie->soc;
> > + if (!soc->pm_support)
> > + return 0;
> > +
> > + list_for_each_entry(port, &pcie->ports, list) {
> > + clk_disable_unprepare(port->ahb_ck);
> > + clk_disable_unprepare(port->sys_ck);
> > + phy_power_off(port->phy);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused mtk_pcie_resume_noirq(struct device *dev)
>
> I don't really like the __maybe_unused annotations. Looking at the
> other users of SET_NOIRQ_SYSTEM_SLEEP_PM_OPS, I think a small majority
> of them wrap the function definitions in #ifdef CONFIG_PM_SLEEP
> instead of using __maybe_unused. That's also a bit ugly, but has the
> advantage of truly omitting the definitions when they're not needed.
Ok, I will follow your advise in the next version.
thanks.
^ permalink raw reply
* Re: [PATCH v9 01/15] ARM: Add Krait L2 register accessor functions
From: Stephen Boyd @ 2018-05-31 7:41 UTC (permalink / raw)
To: Bjorn Andersson, Sricharan R
Cc: robh, viresh.kumar, mark.rutland, mturquette, sboyd, linux,
andy.gross, david.brown, rjw, linux-arm-kernel, devicetree,
linux-kernel, linux-clk, linux-arm-msm, linux-soc, linux-pm,
linux
In-Reply-To: <9f2e1aa8-21c1-10b0-6193-e6bc16993a0d@codeaurora.org>
Quoting Sricharan R (2018-05-30 21:57:20)
> Hi Stephen,
>
> On 5/30/2018 9:25 PM, Stephen Boyd wrote:
> > Quoting Sricharan R (2018-05-24 22:40:11)
> >> Hi Bjorn,
> >>
> >> On 5/24/2018 11:09 PM, Bjorn Andersson wrote:
> >>> On Tue 06 Mar 06:38 PST 2018, Sricharan R wrote:
> >>>
> >>>> From: Stephen Boyd <sboyd@codeaurora.org>
> >>>>
> >>>> Krait CPUs have a handful of L2 cache controller registers that
> >>>> live behind a cp15 based indirection register. First you program
> >>>> the indirection register (l2cpselr) to point the L2 'window'
> >>>> register (l2cpdr) at what you want to read/write. Then you
> >>>> read/write the 'window' register to do what you want. The
> >>>> l2cpselr register is not banked per-cpu so we must lock around
> >>>> accesses to it to prevent other CPUs from re-pointing l2cpdr
> >>>> underneath us.
> >>>>
> >>>> Cc: Mark Rutland <mark.rutland@arm.com>
> >>>> Cc: Russell King <linux@arm.linux.org.uk>
> >>>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> >>>
> >>> This should have your signed-off-by here as well.
> >>>
> >>
> >> ok.
> >>
> >>> Apart from that:
> >>>
> >>> Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> >>>
> >>
> >
> > Will these patches come around again? I'll do a quick sweep on them
> > today but I expect them to be resent.
>
> Sure, i will have to resend them again, fixing couple of Bjorn's
> minor comments. Will address your comments that you would give
> as well along with that.
>
Ok. One general comment is that it would be nice if the bindings for all
the nodes that are introduced included 'clocks' properties and also
maybe 'clock-names' properties for the clocks that are consumed by each
node. Right now, we hide those details from DT and rely on the string
names to hook the clk tree up for us. That sort of prevents us from
moving away from string easily, so I would just throw the clocks into
the binding right now and always have them there just in case we want to
use the binding to figure out the hierarchy in the future.
I've been thinking we need to do something similar for the gcc and other
nodes for any clks they use, but I haven't gotten around to it.
Otherwise the patches look mostly ok to me. Not sure I'll have any other
comments.
^ permalink raw reply
* [PATCH v2] PCI: mediatek: Add system pm support for MT2712
From: honghui.zhang @ 2018-05-31 8:15 UTC (permalink / raw)
To: lorenzo.pieralisi, marc.zyngier, bhelgaas, matthias.bgg,
linux-arm-kernel, linux-mediatek, linux-pci, linux-kernel,
devicetree, yingjoe.chen, eddie.huang, ryder.lee
Cc: honghui.zhang, hongkun.cao, youlin.pei, yong.wu, yt.shen,
sean.wang, xinping.qian
From: Honghui Zhang <honghui.zhang@mediatek.com>
The MTCMOS of PCIe Host for MT2712 will be off when system suspend, and all
the internal control register will be reset after system resume. The PCIe
link should be re-established and the related control register values
should be re-set after system resume.
Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
CC: Ryder Lee <ryder.lee@mediatek.com>
---
drivers/pci/host/pcie-mediatek.c | 61 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
index dabf1086..6bf7f5a 100644
--- a/drivers/pci/host/pcie-mediatek.c
+++ b/drivers/pci/host/pcie-mediatek.c
@@ -132,12 +132,14 @@ struct mtk_pcie_port;
/**
* struct mtk_pcie_soc - differentiate between host generations
* @need_fix_class_id: whether this host's class ID needed to be fixed or not
+ * @pm_support: whether the host's MTCMOS will be off when suspend
* @ops: pointer to configuration access functions
* @startup: pointer to controller setting functions
* @setup_irq: pointer to initialize IRQ functions
*/
struct mtk_pcie_soc {
bool need_fix_class_id;
+ bool pm_support;
struct pci_ops *ops;
int (*startup)(struct mtk_pcie_port *port);
int (*setup_irq)(struct mtk_pcie_port *port, struct device_node *node);
@@ -1179,12 +1181,70 @@ static int mtk_pcie_probe(struct platform_device *pdev)
return err;
}
+#ifdef CONFIG_PM_SLEEP
+static int mtk_pcie_suspend_noirq(struct device *dev)
+{
+ struct mtk_pcie *pcie = dev_get_drvdata(dev);
+ const struct mtk_pcie_soc *soc = pcie->soc;
+ struct mtk_pcie_port *port;
+
+ if (!soc->pm_support)
+ return 0;
+
+ list_for_each_entry(port, &pcie->ports, list) {
+ clk_disable_unprepare(port->ahb_ck);
+ clk_disable_unprepare(port->sys_ck);
+ phy_power_off(port->phy);
+ }
+
+ return 0;
+}
+
+static int mtk_pcie_resume_noirq(struct device *dev)
+{
+ struct mtk_pcie *pcie = dev_get_drvdata(dev);
+ const struct mtk_pcie_soc *soc = pcie->soc;
+ struct mtk_pcie_port *port;
+ int ret;
+
+ soc = pcie->soc;
+ if (!soc->pm_support)
+ return 0;
+
+ list_for_each_entry(port, &pcie->ports, list) {
+ phy_power_on(port->phy);
+ clk_prepare_enable(port->sys_ck);
+ clk_prepare_enable(port->ahb_ck);
+
+ ret = soc->startup(port);
+ if (ret) {
+ dev_err(dev, "Port%d link down\n", port->slot);
+ phy_power_off(port->phy);
+ clk_disable_unprepare(port->sys_ck);
+ clk_disable_unprepare(port->ahb_ck);
+ return ret;
+ }
+
+ if (IS_ENABLED(CONFIG_PCI_MSI))
+ mtk_pcie_enable_msi(port);
+ }
+
+ return 0;
+}
+#endif
+
+const struct dev_pm_ops mtk_pcie_pm_ops = {
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(mtk_pcie_suspend_noirq,
+ mtk_pcie_resume_noirq)
+};
+
static const struct mtk_pcie_soc mtk_pcie_soc_v1 = {
.ops = &mtk_pcie_ops,
.startup = mtk_pcie_startup_port,
};
static const struct mtk_pcie_soc mtk_pcie_soc_mt2712 = {
+ .pm_support = true,
.ops = &mtk_pcie_ops_v2,
.startup = mtk_pcie_startup_port_v2,
.setup_irq = mtk_pcie_setup_irq,
@@ -1211,6 +1271,7 @@ static struct platform_driver mtk_pcie_driver = {
.name = "mtk-pcie",
.of_match_table = mtk_pcie_ids,
.suppress_bind_attrs = true,
+ .pm = &mtk_pcie_pm_ops,
},
};
builtin_platform_driver(mtk_pcie_driver);
--
2.6.4
^ permalink raw reply related
* Re: [PATCH v2 4/8] dt-bindings: gnss: add u-blox binding
From: Johan Hovold @ 2018-05-31 8:22 UTC (permalink / raw)
To: Rob Herring
Cc: Johan Hovold, Greg Kroah-Hartman, Mark Rutland, Andreas Kemnade,
Arnd Bergmann, H . Nikolaus Schaller, Pavel Machek,
Marcel Holtmann, Sebastian Reichel, Tony Lindgren, linux-kernel,
devicetree
In-Reply-To: <20180531035805.GA16906@rob-hp-laptop>
On Wed, May 30, 2018 at 10:58:05PM -0500, Rob Herring wrote:
> On Wed, May 30, 2018 at 12:32:38PM +0200, Johan Hovold wrote:
> > Add binding for u-blox GNSS receivers.
> >
> > Note that the u-blox product names encodes form factor (e.g. "neo"),
> > chipset (e.g. "8") and variant (e.g. "q"), but that only formfactor and
> > chipset is used for the compatible strings (for now).
> >
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > ---
> > .../devicetree/bindings/gnss/u-blox.txt | 44 +++++++++++++++++++
> > .../devicetree/bindings/vendor-prefixes.txt | 1 +
> > 2 files changed, 45 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/gnss/u-blox.txt
> >
> > diff --git a/Documentation/devicetree/bindings/gnss/u-blox.txt b/Documentation/devicetree/bindings/gnss/u-blox.txt
> > new file mode 100644
> > index 000000000000..caef9ace0b0c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gnss/u-blox.txt
> > @@ -0,0 +1,44 @@
> > +u-blox GNSS Receiver DT binding
> > +
> > +The u-blox GNSS receivers can use UART, DDC (I2C), SPI and USB interfaces.
> > +
> > +Please see Documentation/devicetree/bindings/gnss/gnss.txt for generic
> > +properties.
> > +
> > +Required properties:
> > +
> > +- compatible : Must be one of
>
> mixture of space and tab here.
Oops. Same single space character before the tab here in all three
binding docs (and in the in-tree slave_devices.txt which I think I used
as a template).
Do you want me to fix this even if this turns out to be the only thing
that needs to be addressed in a v3?
> > +
> > + "u-blox,neo-8"
> > + "u-blox,neo-m8"
> > +
> > +- vcc-supply : Main voltage regulator
> > +
> > +Required properties (DDC):
> > +- reg : DDC (I2C) slave address
> > +
> > +Required properties (SPI):
> > +- reg : SPI chip select address
> > +
> > +Required properties (USB):
> > +- reg : Number of the USB hub port or the USB host-controller port
> > + to which this device is attached
> > +
> > +Optional properties:
> > +
> > +- timepulse-gpios : Time pulse GPIO
> > +- u-blox,extint-gpios : External interrupt GPIO
>
> This should be interrupts property instead of a gpio.
Contrary to what the name may suggest, this pin is actually an input
which can be used to control active power or to provide time or
frequency aiding data to the receiver (see section 1.13 in [1]).
I only added it for completeness as the driver does not use it
currently. Remove, leave as is, or add "input" to the description as in:
- u-blox,extint-gpios : External interrupt input GPIO
perhaps?
Thanks,
Johan
[1] https://www.u-blox.com/sites/default/files/NEO-8Q_DataSheet_%28UBX-15031913%29.pdf
^ permalink raw reply
* Re: [PATCH v4 3/5] Documentation: DT: add i.MX EPIT timer binding
From: Vladimir Zapolskiy @ 2018-05-31 8:26 UTC (permalink / raw)
To: Clément Péron, Colin Didier, linux-arm-kernel,
devicetree, linux-kernel
Cc: Daniel Lezcano, Thomas Gleixner, Fabio Estevam, Sascha Hauer,
Rob Herring, NXP Linux Team, Pengutronix Kernel Team,
Clément Peron
In-Reply-To: <20180530120327.27681-4-peron.clem@gmail.com>
Hi Clément,
On 05/30/2018 03:03 PM, Clément Péron wrote:
> From: Clément Peron <clement.peron@devialet.com>
>
> Add devicetree binding document for NXP's i.MX SoC specific
> EPIT timer driver.
>
> Signed-off-by: Clément Peron <clement.peron@devialet.com>
> ---
> .../devicetree/bindings/timer/fsl,imxepit.txt | 24 +++++++++++++++++++
> 1 file changed, 24 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/timer/fsl,imxepit.txt
>
> diff --git a/Documentation/devicetree/bindings/timer/fsl,imxepit.txt b/Documentation/devicetree/bindings/timer/fsl,imxepit.txt
> new file mode 100644
> index 000000000000..90112d58af10
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/fsl,imxepit.txt
> @@ -0,0 +1,24 @@
> +Binding for the i.MX EPIT timer
> +
> +This binding uses the common clock binding[1].
> +
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
no, this leftover reference to clock-bindings.txt is invalid, please remove it.
Instead you may add a simple description of the timer module.
> +Required properties:
> +- compatible: should be "fsl,imx31-epit"
To satisfy compatibles with multiple SoCs, apparently you may follow a model,
which is used with other Freescale controllers, for instance
gpio/fsl-imx-gpio.txt - compatible : Should be "fsl,<soc>-gpio"
mmc/fsl-imx-esdhc.txt - compatible : Should be "fsl,<chip>-esdhc"
serial/fsl-imx-uart.txt - compatible : Should be "fsl,<soc>-uart"
timer/fsl,imxgpt.txt - compatible : should be "fsl,<soc>-gpt"
and so on, I hope it would cover Rob's ask.
> +- reg: physical base address of the controller and length of memory mapped
> + region.
> +- interrupts: Should contain EPIT controller interrupt
> +- clocks: list of clock specifiers, must contain an entry for each required
> + entry in clock-names
> +- clock-names : should include entries "ipg", "per"
> +
> +Example for i.MX6QDL:
> + epit1: epit@20d0000 {
> + compatible = "fsl,imx6q-epit", "fsl,imx31-epit";
> + reg = <0x020d0000 0x4000>;
> + interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clks IMX6QDL_CLK_IPG_PER>,
> + <&clks IMX6QDL_CLK_EPIT1>;
> + clock-names = "ipg", "per";
> + };
>
--
With best wishes,
Vladimir
^ permalink raw reply
* [RESEND PATCH v4 0/2] mailbox: introduce STMicroelectronics STM32 IPCC driver
From: Fabien Dessenne @ 2018-05-31 8:27 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Maxime Coquelin, Alexandre Torgue,
Jassi Brar, Ludovic Barre, devicetree, linux-arm-kernel,
linux-kernel
Cc: Benjamin Gaignard, Loic Pallardy, Arnaud Pouliquen
The STMicroelectronics STM32 Inter-Processor Communication Controller
(IPCC) is used for communicating data between two processors.
It provides a non blocking signaling mechanism to post and retrieve
communication data in an atomic way.
Changes since v4:
- add Rob's 'Reviewed-by' in dt bindings
Changes since v3:
- update after Jassi Brar review: remove 'driver.owner'
Changes since v2:
- update bindings and driver according to Rob's comments:
- change compatible property to "st,stm32mp1-ipcc"
- change "st,proc_id" property to "st,proc-id"
- define all interrupts as mandatory
Fabien Dessenne (2):
dt-bindings: mailbox: add STMicroelectronics STM32 IPCC binding
mailbox: add STMicroelectronics STM32 IPCC driver
.../devicetree/bindings/mailbox/stm32-ipcc.txt | 47 +++
drivers/mailbox/Kconfig | 8 +
drivers/mailbox/Makefile | 2 +
drivers/mailbox/stm32-ipcc.c | 402 +++++++++++++++++++++
4 files changed, 459 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/stm32-ipcc.txt
create mode 100644 drivers/mailbox/stm32-ipcc.c
--
2.7.4
^ permalink raw reply
* [RESEND PATCH v4 1/2] dt-bindings: mailbox: add STMicroelectronics STM32 IPCC binding
From: Fabien Dessenne @ 2018-05-31 8:27 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Maxime Coquelin, Alexandre Torgue,
Jassi Brar, Ludovic Barre, devicetree, linux-arm-kernel,
linux-kernel
Cc: Benjamin Gaignard, Loic Pallardy, Arnaud Pouliquen
In-Reply-To: <1527755245-27817-1-git-send-email-fabien.dessenne@st.com>
Add a binding for the STMicroelectronics STM32 IPCC block exposing a
mailbox mechanism between two processors.
Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/mailbox/stm32-ipcc.txt | 47 ++++++++++++++++++++++
1 file changed, 47 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/stm32-ipcc.txt
diff --git a/Documentation/devicetree/bindings/mailbox/stm32-ipcc.txt b/Documentation/devicetree/bindings/mailbox/stm32-ipcc.txt
new file mode 100644
index 0000000..1d2b7fe
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/stm32-ipcc.txt
@@ -0,0 +1,47 @@
+* STMicroelectronics STM32 IPCC (Inter-Processor Communication Controller)
+
+The IPCC block provides a non blocking signaling mechanism to post and
+retrieve messages in an atomic way between two processors.
+It provides the signaling for N bidirectionnal channels. The number of channels
+(N) can be read from a dedicated register.
+
+Required properties:
+- compatible: Must be "st,stm32mp1-ipcc"
+- reg: Register address range (base address and length)
+- st,proc-id: Processor id using the mailbox (0 or 1)
+- clocks: Input clock
+- interrupt-names: List of names for the interrupts described by the interrupt
+ property. Must contain the following entries:
+ - "rx"
+ - "tx"
+ - "wakeup"
+- interrupts: Interrupt specifiers for "rx channel occupied", "tx channel
+ free" and "system wakeup".
+- #mbox-cells: Number of cells required for the mailbox specifier. Must be 1.
+ The data contained in the mbox specifier of the "mboxes"
+ property in the client node is the mailbox channel index.
+
+Optional properties:
+- wakeup-source: Flag to indicate whether this device can wake up the system
+
+
+
+Example:
+ ipcc: mailbox@4c001000 {
+ compatible = "st,stm32mp1-ipcc";
+ #mbox-cells = <1>;
+ reg = <0x4c001000 0x400>;
+ st,proc-id = <0>;
+ interrupts-extended = <&intc GIC_SPI 100 IRQ_TYPE_NONE>,
+ <&intc GIC_SPI 101 IRQ_TYPE_NONE>,
+ <&aiec 62 1>;
+ interrupt-names = "rx", "tx", "wakeup";
+ clocks = <&rcc_clk IPCC>;
+ wakeup-source;
+ }
+
+Client:
+ mbox_test {
+ ...
+ mboxes = <&ipcc 0>, <&ipcc 1>;
+ };
--
2.7.4
^ permalink raw reply related
* [RESEND PATCH v4 2/2] mailbox: add STMicroelectronics STM32 IPCC driver
From: Fabien Dessenne @ 2018-05-31 8:27 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Maxime Coquelin, Alexandre Torgue,
Jassi Brar, Ludovic Barre, devicetree, linux-arm-kernel,
linux-kernel
Cc: Benjamin Gaignard, Loic Pallardy, Arnaud Pouliquen
In-Reply-To: <1527755245-27817-1-git-send-email-fabien.dessenne@st.com>
The STMicroelectronics STM32 Inter-Processor Communication Controller
(IPCC) is used for communicating data between two processors.
It provides a non blocking signaling mechanism to post and retrieve
communication data in an atomic way.
Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
drivers/mailbox/Kconfig | 8 +
drivers/mailbox/Makefile | 2 +
drivers/mailbox/stm32-ipcc.c | 402 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 412 insertions(+)
create mode 100644 drivers/mailbox/stm32-ipcc.c
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index ba2f152..d7581f0 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -171,4 +171,12 @@ config BCM_FLEXRM_MBOX
Mailbox implementation of the Broadcom FlexRM ring manager,
which provides access to various offload engines on Broadcom
SoCs. Say Y here if you want to use the Broadcom FlexRM.
+
+config STM32_IPCC
+ tristate "STM32 IPCC Mailbox"
+ depends on MACH_STM32MP157
+ help
+ Mailbox implementation for STMicroelectonics STM32 family chips
+ with hardware for Inter-Processor Communication Controller (IPCC)
+ between processors. Say Y here if you want to have this support.
endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 4896f8d..7ea9654 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -36,3 +36,5 @@ obj-$(CONFIG_BCM_FLEXRM_MBOX) += bcm-flexrm-mailbox.o
obj-$(CONFIG_QCOM_APCS_IPC) += qcom-apcs-ipc-mailbox.o
obj-$(CONFIG_TEGRA_HSP_MBOX) += tegra-hsp.o
+
+obj-$(CONFIG_STM32_IPCC) += stm32-ipcc.o
diff --git a/drivers/mailbox/stm32-ipcc.c b/drivers/mailbox/stm32-ipcc.c
new file mode 100644
index 0000000..533b0da
--- /dev/null
+++ b/drivers/mailbox/stm32-ipcc.c
@@ -0,0 +1,402 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) STMicroelectronics 2018 - All Rights Reserved
+ * Authors: Ludovic Barre <ludovic.barre@st.com> for STMicroelectronics.
+ * Fabien Dessenne <fabien.dessenne@st.com> for STMicroelectronics.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/pm_wakeirq.h>
+
+#define IPCC_XCR 0x000
+#define XCR_RXOIE BIT(0)
+#define XCR_TXOIE BIT(16)
+
+#define IPCC_XMR 0x004
+#define IPCC_XSCR 0x008
+#define IPCC_XTOYSR 0x00c
+
+#define IPCC_PROC_OFFST 0x010
+
+#define IPCC_HWCFGR 0x3f0
+#define IPCFGR_CHAN_MASK GENMASK(7, 0)
+
+#define IPCC_VER 0x3f4
+#define VER_MINREV_MASK GENMASK(3, 0)
+#define VER_MAJREV_MASK GENMASK(7, 4)
+
+#define RX_BIT_MASK GENMASK(15, 0)
+#define RX_BIT_CHAN(chan) BIT(chan)
+#define TX_BIT_SHIFT 16
+#define TX_BIT_MASK GENMASK(31, 16)
+#define TX_BIT_CHAN(chan) BIT(TX_BIT_SHIFT + (chan))
+
+#define STM32_MAX_PROCS 2
+
+enum {
+ IPCC_IRQ_RX,
+ IPCC_IRQ_TX,
+ IPCC_IRQ_NUM,
+};
+
+struct stm32_ipcc {
+ struct mbox_controller controller;
+ void __iomem *reg_base;
+ void __iomem *reg_proc;
+ struct clk *clk;
+ int irqs[IPCC_IRQ_NUM];
+ int wkp;
+ u32 proc_id;
+ u32 n_chans;
+ u32 xcr;
+ u32 xmr;
+};
+
+static inline void stm32_ipcc_set_bits(void __iomem *reg, u32 mask)
+{
+ writel_relaxed(readl_relaxed(reg) | mask, reg);
+}
+
+static inline void stm32_ipcc_clr_bits(void __iomem *reg, u32 mask)
+{
+ writel_relaxed(readl_relaxed(reg) & ~mask, reg);
+}
+
+static irqreturn_t stm32_ipcc_rx_irq(int irq, void *data)
+{
+ struct stm32_ipcc *ipcc = data;
+ struct device *dev = ipcc->controller.dev;
+ u32 status, mr, tosr, chan;
+ irqreturn_t ret = IRQ_NONE;
+ int proc_offset;
+
+ /* read 'channel occupied' status from other proc */
+ proc_offset = ipcc->proc_id ? -IPCC_PROC_OFFST : IPCC_PROC_OFFST;
+ tosr = readl_relaxed(ipcc->reg_proc + proc_offset + IPCC_XTOYSR);
+ mr = readl_relaxed(ipcc->reg_proc + IPCC_XMR);
+
+ /* search for unmasked 'channel occupied' */
+ status = tosr & FIELD_GET(RX_BIT_MASK, ~mr);
+
+ for (chan = 0; chan < ipcc->n_chans; chan++) {
+ if (!(status & (1 << chan)))
+ continue;
+
+ dev_dbg(dev, "%s: chan:%d rx\n", __func__, chan);
+
+ mbox_chan_received_data(&ipcc->controller.chans[chan], NULL);
+
+ stm32_ipcc_set_bits(ipcc->reg_proc + IPCC_XSCR,
+ RX_BIT_CHAN(chan));
+
+ ret = IRQ_HANDLED;
+ }
+
+ return ret;
+}
+
+static irqreturn_t stm32_ipcc_tx_irq(int irq, void *data)
+{
+ struct stm32_ipcc *ipcc = data;
+ struct device *dev = ipcc->controller.dev;
+ u32 status, mr, tosr, chan;
+ irqreturn_t ret = IRQ_NONE;
+
+ tosr = readl_relaxed(ipcc->reg_proc + IPCC_XTOYSR);
+ mr = readl_relaxed(ipcc->reg_proc + IPCC_XMR);
+
+ /* search for unmasked 'channel free' */
+ status = ~tosr & FIELD_GET(TX_BIT_MASK, ~mr);
+
+ for (chan = 0; chan < ipcc->n_chans ; chan++) {
+ if (!(status & (1 << chan)))
+ continue;
+
+ dev_dbg(dev, "%s: chan:%d tx\n", __func__, chan);
+
+ /* mask 'tx channel free' interrupt */
+ stm32_ipcc_set_bits(ipcc->reg_proc + IPCC_XMR,
+ TX_BIT_CHAN(chan));
+
+ mbox_chan_txdone(&ipcc->controller.chans[chan], 0);
+
+ ret = IRQ_HANDLED;
+ }
+
+ return ret;
+}
+
+static int stm32_ipcc_send_data(struct mbox_chan *link, void *data)
+{
+ unsigned int chan = (unsigned int)link->con_priv;
+ struct stm32_ipcc *ipcc = container_of(link->mbox, struct stm32_ipcc,
+ controller);
+
+ dev_dbg(ipcc->controller.dev, "%s: chan:%d\n", __func__, chan);
+
+ /* set channel n occupied */
+ stm32_ipcc_set_bits(ipcc->reg_proc + IPCC_XSCR, TX_BIT_CHAN(chan));
+
+ /* unmask 'tx channel free' interrupt */
+ stm32_ipcc_clr_bits(ipcc->reg_proc + IPCC_XMR, TX_BIT_CHAN(chan));
+
+ return 0;
+}
+
+static int stm32_ipcc_startup(struct mbox_chan *link)
+{
+ unsigned int chan = (unsigned int)link->con_priv;
+ struct stm32_ipcc *ipcc = container_of(link->mbox, struct stm32_ipcc,
+ controller);
+ int ret;
+
+ ret = clk_prepare_enable(ipcc->clk);
+ if (ret) {
+ dev_err(ipcc->controller.dev, "can not enable the clock\n");
+ return ret;
+ }
+
+ /* unmask 'rx channel occupied' interrupt */
+ stm32_ipcc_clr_bits(ipcc->reg_proc + IPCC_XMR, RX_BIT_CHAN(chan));
+
+ return 0;
+}
+
+static void stm32_ipcc_shutdown(struct mbox_chan *link)
+{
+ unsigned int chan = (unsigned int)link->con_priv;
+ struct stm32_ipcc *ipcc = container_of(link->mbox, struct stm32_ipcc,
+ controller);
+
+ /* mask rx/tx interrupt */
+ stm32_ipcc_set_bits(ipcc->reg_proc + IPCC_XMR,
+ RX_BIT_CHAN(chan) | TX_BIT_CHAN(chan));
+
+ clk_disable_unprepare(ipcc->clk);
+}
+
+static const struct mbox_chan_ops stm32_ipcc_ops = {
+ .send_data = stm32_ipcc_send_data,
+ .startup = stm32_ipcc_startup,
+ .shutdown = stm32_ipcc_shutdown,
+};
+
+static int stm32_ipcc_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ struct stm32_ipcc *ipcc;
+ struct resource *res;
+ unsigned int i;
+ int ret;
+ u32 ip_ver;
+ static const char * const irq_name[] = {"rx", "tx"};
+ irq_handler_t irq_thread[] = {stm32_ipcc_rx_irq, stm32_ipcc_tx_irq};
+
+ if (!np) {
+ dev_err(dev, "No DT found\n");
+ return -ENODEV;
+ }
+
+ ipcc = devm_kzalloc(dev, sizeof(*ipcc), GFP_KERNEL);
+ if (!ipcc)
+ return -ENOMEM;
+
+ /* proc_id */
+ if (of_property_read_u32(np, "st,proc-id", &ipcc->proc_id)) {
+ dev_err(dev, "Missing st,proc-id\n");
+ return -ENODEV;
+ }
+
+ if (ipcc->proc_id >= STM32_MAX_PROCS) {
+ dev_err(dev, "Invalid proc_id (%d)\n", ipcc->proc_id);
+ return -EINVAL;
+ }
+
+ /* regs */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ ipcc->reg_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(ipcc->reg_base))
+ return PTR_ERR(ipcc->reg_base);
+
+ ipcc->reg_proc = ipcc->reg_base + ipcc->proc_id * IPCC_PROC_OFFST;
+
+ /* clock */
+ ipcc->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(ipcc->clk))
+ return PTR_ERR(ipcc->clk);
+
+ ret = clk_prepare_enable(ipcc->clk);
+ if (ret) {
+ dev_err(dev, "can not enable the clock\n");
+ return ret;
+ }
+
+ /* irq */
+ for (i = 0; i < IPCC_IRQ_NUM; i++) {
+ ipcc->irqs[i] = of_irq_get_byname(dev->of_node, irq_name[i]);
+ if (ipcc->irqs[i] < 0) {
+ dev_err(dev, "no IRQ specified %s\n", irq_name[i]);
+ ret = ipcc->irqs[i];
+ goto err_clk;
+ }
+
+ ret = devm_request_threaded_irq(dev, ipcc->irqs[i], NULL,
+ irq_thread[i], IRQF_ONESHOT,
+ dev_name(dev), ipcc);
+ if (ret) {
+ dev_err(dev, "failed to request irq %d (%d)\n", i, ret);
+ goto err_clk;
+ }
+ }
+
+ /* mask and enable rx/tx irq */
+ stm32_ipcc_set_bits(ipcc->reg_proc + IPCC_XMR,
+ RX_BIT_MASK | TX_BIT_MASK);
+ stm32_ipcc_set_bits(ipcc->reg_proc + IPCC_XCR, XCR_RXOIE | XCR_TXOIE);
+
+ /* wakeup */
+ if (of_property_read_bool(np, "wakeup-source")) {
+ ipcc->wkp = of_irq_get_byname(dev->of_node, "wakeup");
+ if (ipcc->wkp < 0) {
+ dev_err(dev, "could not get wakeup IRQ\n");
+ ret = ipcc->wkp;
+ goto err_clk;
+ }
+
+ device_init_wakeup(dev, true);
+ ret = dev_pm_set_dedicated_wake_irq(dev, ipcc->wkp);
+ if (ret) {
+ dev_err(dev, "Failed to set wake up irq\n");
+ goto err_init_wkp;
+ }
+ } else {
+ device_init_wakeup(dev, false);
+ }
+
+ /* mailbox controller */
+ ipcc->n_chans = readl_relaxed(ipcc->reg_base + IPCC_HWCFGR);
+ ipcc->n_chans &= IPCFGR_CHAN_MASK;
+
+ ipcc->controller.dev = dev;
+ ipcc->controller.txdone_irq = true;
+ ipcc->controller.ops = &stm32_ipcc_ops;
+ ipcc->controller.num_chans = ipcc->n_chans;
+ ipcc->controller.chans = devm_kcalloc(dev, ipcc->controller.num_chans,
+ sizeof(*ipcc->controller.chans),
+ GFP_KERNEL);
+ if (!ipcc->controller.chans) {
+ ret = -ENOMEM;
+ goto err_irq_wkp;
+ }
+
+ for (i = 0; i < ipcc->controller.num_chans; i++)
+ ipcc->controller.chans[i].con_priv = (void *)i;
+
+ ret = mbox_controller_register(&ipcc->controller);
+ if (ret)
+ goto err_irq_wkp;
+
+ platform_set_drvdata(pdev, ipcc);
+
+ ip_ver = readl_relaxed(ipcc->reg_base + IPCC_VER);
+
+ dev_info(dev, "ipcc rev:%ld.%ld enabled, %d chans, proc %d\n",
+ FIELD_GET(VER_MAJREV_MASK, ip_ver),
+ FIELD_GET(VER_MINREV_MASK, ip_ver),
+ ipcc->controller.num_chans, ipcc->proc_id);
+
+ clk_disable_unprepare(ipcc->clk);
+ return 0;
+
+err_irq_wkp:
+ if (ipcc->wkp)
+ dev_pm_clear_wake_irq(dev);
+err_init_wkp:
+ device_init_wakeup(dev, false);
+err_clk:
+ clk_disable_unprepare(ipcc->clk);
+ return ret;
+}
+
+static int stm32_ipcc_remove(struct platform_device *pdev)
+{
+ struct stm32_ipcc *ipcc = platform_get_drvdata(pdev);
+
+ mbox_controller_unregister(&ipcc->controller);
+
+ if (ipcc->wkp)
+ dev_pm_clear_wake_irq(&pdev->dev);
+
+ device_init_wakeup(&pdev->dev, false);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static void stm32_ipcc_set_irq_wake(struct device *dev, bool enable)
+{
+ struct stm32_ipcc *ipcc = dev_get_drvdata(dev);
+ unsigned int i;
+
+ if (device_may_wakeup(dev))
+ for (i = 0; i < IPCC_IRQ_NUM; i++)
+ irq_set_irq_wake(ipcc->irqs[i], enable);
+}
+
+static int stm32_ipcc_suspend(struct device *dev)
+{
+ struct stm32_ipcc *ipcc = dev_get_drvdata(dev);
+
+ ipcc->xmr = readl_relaxed(ipcc->reg_proc + IPCC_XMR);
+ ipcc->xcr = readl_relaxed(ipcc->reg_proc + IPCC_XCR);
+
+ stm32_ipcc_set_irq_wake(dev, true);
+
+ return 0;
+}
+
+static int stm32_ipcc_resume(struct device *dev)
+{
+ struct stm32_ipcc *ipcc = dev_get_drvdata(dev);
+
+ stm32_ipcc_set_irq_wake(dev, false);
+
+ writel_relaxed(ipcc->xmr, ipcc->reg_proc + IPCC_XMR);
+ writel_relaxed(ipcc->xcr, ipcc->reg_proc + IPCC_XCR);
+
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(stm32_ipcc_pm_ops,
+ stm32_ipcc_suspend, stm32_ipcc_resume);
+
+static const struct of_device_id stm32_ipcc_of_match[] = {
+ { .compatible = "st,stm32mp1-ipcc" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, stm32_ipcc_of_match);
+
+static struct platform_driver stm32_ipcc_driver = {
+ .driver = {
+ .name = "stm32-ipcc",
+ .pm = &stm32_ipcc_pm_ops,
+ .of_match_table = stm32_ipcc_of_match,
+ },
+ .probe = stm32_ipcc_probe,
+ .remove = stm32_ipcc_remove,
+};
+
+module_platform_driver(stm32_ipcc_driver);
+
+MODULE_AUTHOR("Ludovic Barre <ludovic.barre@st.com>");
+MODULE_AUTHOR("Fabien Dessenne <fabien.dessenne@st.com>");
+MODULE_DESCRIPTION("STM32 IPCC driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v4 5/5] ARM: dts: imx6qdl: add missing compatible and clock properties for EPIT
From: Vladimir Zapolskiy @ 2018-05-31 8:33 UTC (permalink / raw)
To: Clément Péron, Colin Didier, linux-arm-kernel,
devicetree, linux-kernel
Cc: Rob Herring, Sascha Hauer, Daniel Lezcano, Clément Peron,
NXP Linux Team, Pengutronix Kernel Team, Fabio Estevam,
Thomas Gleixner
In-Reply-To: <20180530120327.27681-6-peron.clem@gmail.com>
On 05/30/2018 03:03 PM, Clément Péron wrote:
> From: Colin Didier <colin.didier@devialet.com>
>
> Add missing compatible and clock properties for EPIT node.
>
> Signed-off-by: Colin Didier <colin.didier@devialet.com>
> Signed-off-by: Clément Peron <clement.peron@devialet.com>
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> arch/arm/boot/dts/imx6qdl.dtsi | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index c003e62bf290..0feec516847a 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -844,13 +844,23 @@
> };
>
> epit1: epit@20d0000 { /* EPIT1 */
epit1: timer@20d0000 { ...
And /* EPIT1 */ comment can be removed, it is quite clear from the same
line context.
Formally it is a subject to another patch, but I think this can be
accepted as a part of this one.
> + compatible = "fsl,imx6q-epit", "fsl,imx31-epit";
> reg = <0x020d0000 0x4000>;
> interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clks IMX6QDL_CLK_IPG_PER>,
> + <&clks IMX6QDL_CLK_EPIT1>;
> + clock-names = "ipg", "per";
> + status = "disabled";
> };
>
> epit2: epit@20d4000 { /* EPIT2 */
Same as above.
> + compatible = "fsl,imx6q-epit", "fsl,imx31-epit";
> reg = <0x020d4000 0x4000>;
> interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clks IMX6QDL_CLK_IPG_PER>,
> + <&clks IMX6QDL_CLK_EPIT2>;
> + clock-names = "ipg", "per";
> + status = "disabled";
> };
>
> src: src@20d8000 {
>
--
With best wishes,
Vladimir
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 4/5] clocksource: add driver for i.MX EPIT timer
From: Vladimir Zapolskiy @ 2018-05-31 8:36 UTC (permalink / raw)
To: Clément Péron, Colin Didier, linux-arm-kernel,
devicetree, linux-kernel
Cc: Daniel Lezcano, Thomas Gleixner, Fabio Estevam, Sascha Hauer,
Rob Herring, NXP Linux Team, Pengutronix Kernel Team,
Clément Peron
In-Reply-To: <20180530120327.27681-5-peron.clem@gmail.com>
Hi Clément,
On 05/30/2018 03:03 PM, Clément Péron wrote:
> From: Colin Didier <colin.didier@devialet.com>
>
> Add driver for NXP's EPIT timer used in i.MX 6 family of SoC.
>
> Signed-off-by: Colin Didier <colin.didier@devialet.com>
> Signed-off-by: Clément Peron <clement.peron@devialet.com>
> ---
[snip]
> +++ b/drivers/clocksource/timer-imx-epit.c
> @@ -0,0 +1,281 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * i.MX EPIT Timer
> + *
> + * Copyright (C) 2010 Sascha Hauer <s.hauer@pengutronix.de>
> + * Copyright (C) 2018 Colin Didier <colin.didier@devialet.com>
> + * Copyright (C) 2018 Clément Péron <clement.peron@devialet.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clockchips.h>
> +#include <linux/err.h>
The included header above still can be removed.
I have no more comments about the code, I will try to find time to
test the driver, but please don't take it as a promise.
--
With best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH v4 5/5] ARM: dts: imx6qdl: add missing compatible and clock properties for EPIT
From: Clément Péron @ 2018-05-31 8:41 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Colin Didier, linux-arm-kernel, devicetree, linux-kernel,
Daniel Lezcano, Thomas Gleixner, Fabio Estevam, Sascha Hauer,
Rob Herring, NXP Linux Team, Pengutronix Kernel Team,
Clément Peron
In-Reply-To: <a4042956-24e1-6132-4606-b5b8f4e30023@mentor.com>
Hi Vladimir,
On Thu, 31 May 2018 at 10:33, Vladimir Zapolskiy
<vladimir_zapolskiy@mentor.com> wrote:
>
> On 05/30/2018 03:03 PM, Clément Péron wrote:
> > From: Colin Didier <colin.didier@devialet.com>
> >
> > Add missing compatible and clock properties for EPIT node.
> >
> > Signed-off-by: Colin Didier <colin.didier@devialet.com>
> > Signed-off-by: Clément Peron <clement.peron@devialet.com>
> > Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
> > ---
> > arch/arm/boot/dts/imx6qdl.dtsi | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> > index c003e62bf290..0feec516847a 100644
> > --- a/arch/arm/boot/dts/imx6qdl.dtsi
> > +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> > @@ -844,13 +844,23 @@
> > };
> >
> > epit1: epit@20d0000 { /* EPIT1 */
>
> epit1: timer@20d0000 { ...
>
> And /* EPIT1 */ comment can be removed, it is quite clear from the same
> line context.
>
> Formally it is a subject to another patch, but I think this can be
> accepted as a part of this one.
Should I also update other boards ?
I only did it for imx6qdl.dtsi, but the EPIT is present in other boards
but i can't test it myself.
>
> > + compatible = "fsl,imx6q-epit", "fsl,imx31-epit";
> > reg = <0x020d0000 0x4000>;
> > interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clks IMX6QDL_CLK_IPG_PER>,
> > + <&clks IMX6QDL_CLK_EPIT1>;
> > + clock-names = "ipg", "per";
> > + status = "disabled";
> > };
> >
> > epit2: epit@20d4000 { /* EPIT2 */
>
> Same as above.
>
> > + compatible = "fsl,imx6q-epit", "fsl,imx31-epit";
> > reg = <0x020d4000 0x4000>;
> > interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clks IMX6QDL_CLK_IPG_PER>,
> > + <&clks IMX6QDL_CLK_EPIT2>;
> > + clock-names = "ipg", "per";
> > + status = "disabled";
> > };
> >
> > src: src@20d8000 {
> >
>
> --
> With best wishes,
> Vladimir
^ permalink raw reply
* Re: [PATCH v2 0/8] gnss: add new GNSS subsystem
From: Johan Hovold @ 2018-05-31 8:52 UTC (permalink / raw)
To: Richard Cochran
Cc: Johan Hovold, Greg Kroah-Hartman, Rob Herring, Mark Rutland,
Andreas Kemnade, Arnd Bergmann, H . Nikolaus Schaller,
Pavel Machek, Marcel Holtmann, Sebastian Reichel, Tony Lindgren,
linux-kernel, devicetree
In-Reply-To: <20180530143822.lvtwjvbqe7gvbvgq@localhost>
On Wed, May 30, 2018 at 07:38:22AM -0700, Richard Cochran wrote:
> On Wed, May 30, 2018 at 12:32:34PM +0200, Johan Hovold wrote:
> > Another possible extension is to add generic 1PPS support.
>
> There are two possibilities to consider.
>
> 1. If the PPS causes an interrupt, then it should hook into the PPS
> subsystem.
Registering a PPS child device is what I had in mind for this.
> 2. If the PPS is a HW signal routed for example to the input pin of a
> MAC based PTP Hardware Clock (PHC), then it would make sense to
> model the GNSS device as a PHC as well. This PHC would be readable
> but not writable, and more importantly it would offer an output
> signal. Then user space could use the existing interfaces to
> dial the PPS signal from one device the another.
Ok.
> (Come to think of it, modeling the GNSS as PHC would also give you the
> interface for #1 as well.)
Thanks
Johan
^ permalink raw reply
* Re: [PATCH v4 5/5] ARM: dts: imx6qdl: add missing compatible and clock properties for EPIT
From: Vladimir Zapolskiy @ 2018-05-31 8:54 UTC (permalink / raw)
To: Clément Péron
Cc: Colin Didier, linux-arm-kernel, devicetree, linux-kernel,
Daniel Lezcano, Thomas Gleixner, Fabio Estevam, Sascha Hauer,
Rob Herring, NXP Linux Team, Pengutronix Kernel Team,
Clément Peron
In-Reply-To: <CAJiuCcefvMg6r=06O8m1TYa0hat_dHydcoJ7PN58qUru1K8zMQ@mail.gmail.com>
Hi Clément,
On 05/31/2018 11:41 AM, Clément Péron wrote:
> Hi Vladimir,
>
> On Thu, 31 May 2018 at 10:33, Vladimir Zapolskiy
> <vladimir_zapolskiy@mentor.com> wrote:
>>
>> On 05/30/2018 03:03 PM, Clément Péron wrote:
>>> From: Colin Didier <colin.didier@devialet.com>
>>>
>>> Add missing compatible and clock properties for EPIT node.
>>>
>>> Signed-off-by: Colin Didier <colin.didier@devialet.com>
>>> Signed-off-by: Clément Peron <clement.peron@devialet.com>
>>> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
>>> ---
>>> arch/arm/boot/dts/imx6qdl.dtsi | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
>>> index c003e62bf290..0feec516847a 100644
>>> --- a/arch/arm/boot/dts/imx6qdl.dtsi
>>> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
>>> @@ -844,13 +844,23 @@
>>> };
>>>
>>> epit1: epit@20d0000 { /* EPIT1 */
>>
>> epit1: timer@20d0000 { ...
>>
>> And /* EPIT1 */ comment can be removed, it is quite clear from the same
>> line context.
>>
>> Formally it is a subject to another patch, but I think this can be
>> accepted as a part of this one.
>
> Should I also update other boards ?
> I only did it for imx6qdl.dtsi, but the EPIT is present in other boards
> but i can't test it myself.
>
Sure, please do it, why not, it is quite a safe modification.
One change per one dtsi file will suffice, and I see that imx25.dtsi
already contains the requested change, however probably you may
want to update its compatible = "fsl,imx25-epit" line.
Regarding compatibles for other imx6* SoCs, I think all of them should
be documented in fsl,imxepit.txt and then added to the correspondent
dtsi files one per SoC.
And I forgot the outcome of one former discussion with Uwe Kleine-König,
but if my bad memory serves me, we agreed that i.MX25 was released later
than i.MX31, so the most generic (the last value in the list) compatible
should be a compatible with i.MX31 like in
imx25.dtsi:367: compatible = "fsl,imx25-gpt", "fsl,imx31-gpt";
--
With best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH 11/11] misc/throttler: Add Chrome OS EC throttler
From: Enric Balletbo Serra @ 2018-05-31 9:05 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Arnd Bergmann,
Greg Kroah-Hartman, Rob Herring, Mark Rutland, Linux PM list,
devicetree@vger.kernel.org, linux-kernel, Brian Norris,
Douglas Anderson
In-Reply-To: <20180525203043.249193-12-mka@chromium.org>
Hi Matthias,
The patch looks good to me, just three minor comments to be more
coherent with other cros-ec drivers.
2018-05-25 22:30 GMT+02:00 Matthias Kaehlcke <mka@chromium.org>:
> The driver subscribes to throttling events from the Chrome OS
> embedded controller and enables/disables system throttling based
> on these events.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> drivers/misc/throttler/Kconfig | 15 +++
> drivers/misc/throttler/Makefile | 1 +
> drivers/misc/throttler/cros_ec_throttler.c | 122 +++++++++++++++++++++
> 3 files changed, 138 insertions(+)
> create mode 100644 drivers/misc/throttler/cros_ec_throttler.c
>
> diff --git a/drivers/misc/throttler/Kconfig b/drivers/misc/throttler/Kconfig
> index ef8388f6bc0a..652b6817b75c 100644
> --- a/drivers/misc/throttler/Kconfig
> +++ b/drivers/misc/throttler/Kconfig
> @@ -11,3 +11,18 @@ menuconfig THROTTLER
> Note that you also need a event monitor module usually called
> *_throttler.
>
> +if THROTTLER
> +
> +config CROS_EC_THROTTLER
> + tristate "Throttler event monitor for the Chrome OS Embedded Controller"
s/Chrome OS/ChromeOS/ This is how other cros-ec drivers refer to the
embedded controller, so will be good if you could maintain this
denomination.
> + default n
> + depends on MFD_CROS_EC
> + ---help---
> + This driver adds support to throttle the system in reaction to
> + Chrome OS EC events.
> +
ditto
> + To compile this driver as a module, choose M here: the
> + module will be called cros_ec_throttler.
> +
> +endif # THROTTLER
> +
> diff --git a/drivers/misc/throttler/Makefile b/drivers/misc/throttler/Makefile
> index c8d920cee315..d9b2a77dabc9 100644
> --- a/drivers/misc/throttler/Makefile
> +++ b/drivers/misc/throttler/Makefile
> @@ -1 +1,2 @@
> obj-$(CONFIG_THROTTLER) += core.o
> +obj-$(CONFIG_CROS_EC_THROTTLER) += cros_ec_throttler.o
> diff --git a/drivers/misc/throttler/cros_ec_throttler.c b/drivers/misc/throttler/cros_ec_throttler.c
> new file mode 100644
> index 000000000000..ea6bc002d49c
> --- /dev/null
> +++ b/drivers/misc/throttler/cros_ec_throttler.c
> @@ -0,0 +1,122 @@
> +/*
> + * Driver for throttling triggered by EC events.
> + *
> + * Copyright (C) 2018 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.
> + *
> + */
> +
Replace this for the SPDX header format
// SPDX-License-Identifier: GPL-2.0+
// Driver for throttling triggered by EC events.
//
// Copyright (C) 2018 Google, Inc.
// Author: Matthias Kaehlcke <mka@chromium.org>
> +#include <linux/kernel.h>
> +#include <linux/mfd/cros_ec.h>
> +#include <linux/module.h>
> +#include <linux/notifier.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/throttler.h>
> +
> +struct cros_ec_throttler {
> + struct cros_ec_device *ec;
> + struct throttler *throttler;
> + struct notifier_block nb;
> +};
> +
> +static int cros_ec_throttler_event(struct notifier_block *nb,
> + unsigned long queued_during_suspend, void *_notify)
> +{
> + struct cros_ec_throttler *cte =
> + container_of(nb, struct cros_ec_throttler, nb);
nit: Better add a define here instead of split the line like this ?
> + u32 host_event;
> +
> + host_event = cros_ec_get_host_event(cte->ec);
> + if (host_event & EC_HOST_EVENT_MASK(EC_HOST_EVENT_THROTTLE_START)) {
> + throttler_set_level(cte->throttler, 1);
> +
> + return NOTIFY_OK;
> + } else if (host_event & EC_HOST_EVENT_MASK(EC_HOST_EVENT_THROTTLE_STOP)) {
> + throttler_set_level(cte->throttler, 0);
> +
> + return NOTIFY_OK;
> + }
> +
> + return NOTIFY_DONE;
> +}
> +
> +static int cros_ec_throttler_probe(struct platform_device *pdev)
> +{
> + struct cros_ec_throttler *cte;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = pdev->dev.of_node;
> + int ret;
> +
> + if (!np) {
> + /* should never happen */
> + return -EINVAL;
> + }
> +
> + cte = devm_kzalloc(dev, sizeof(*cte), GFP_KERNEL);
> + if (!cte)
> + return -ENOMEM;
> +
> + cte->ec = dev_get_drvdata(pdev->dev.parent);
> +
> + cte->throttler = throttler_setup(dev);
> + if (IS_ERR(cte->throttler))
> + return PTR_ERR(cte->throttler);
> +
> + dev_set_drvdata(dev, cte);
> +
> + cte->nb.notifier_call = cros_ec_throttler_event;
> + ret = blocking_notifier_chain_register(&cte->ec->event_notifier,
> + &cte->nb);
> + if (ret < 0) {
> + dev_err(dev, "failed to register notifier\n");
> + throttler_teardown(cte->throttler);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int cros_ec_throttler_remove(struct platform_device *pdev)
> +{
> + struct cros_ec_throttler *cte = platform_get_drvdata(pdev);
> +
> + blocking_notifier_chain_unregister(&cte->ec->event_notifier,
> + &cte->nb);
> +
> + throttler_teardown(cte->throttler);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id cros_ec_throttler_of_match[] = {
> + { .compatible = "google,cros-ec-throttler" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, cros_ec_throttler_of_match);
> +#endif /* CONFIG_OF */
> +
> +static struct platform_driver cros_ec_throttler_driver = {
> + .driver = {
> + .name = "cros-ec-throttler",
> + .of_match_table = of_match_ptr(cros_ec_throttler_of_match),
> + },
> + .probe = cros_ec_throttler_probe,
> + .remove = cros_ec_throttler_remove,
> +};
> +
> +module_platform_driver(cros_ec_throttler_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Matthias Kaehlcke <mka@chromium.org>");
> +MODULE_DESCRIPTION("Chrome OS EC Throttler");
s/Chrome OS/ChromeOS/
> --
> 2.17.0.921.gf22659ad46-goog
>
Reviewed-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Thanks,
Enric
^ permalink raw reply
* Re: [PATCH v7 1/5] dt-bindings: Add the r9a06g032-sysctrl.h file
From: M P @ 2018-05-31 9:11 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: michel.pollet, linux-renesas-soc, Simon Horman, Phil Edworthy,
buserror+upstream, Michael Turquette, sboyd, robh+dt,
mark.rutland, geert+renesas, linux-clk, devicetree, linux-kernel
In-Reply-To: <CAMuHMdVzN0Cbc9+MJqC4EzVgD3bkDixkf10NA13ko87cMwvpLA@mail.gmail.com>
On Fri, 25 May 2018 at 11:31, Geert Uytterhoeven <geert@linux-m68k.org>
wrote:
> Hi Michel,
Hi Geert,
> On Thu, May 24, 2018 at 11:28 AM, Michel Pollet
> <michel.pollet@bp.renesas.com> wrote:
> > This adds the constants necessary to use the renesas,r9a06g032-sysctrl
node.
> >
> > Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
> Thanks for your patch!
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/r9a06g032-sysctrl.h
> You can still call this file r9a06g032-clocks.h, if you want, as it
> contains clock definitions only.
Well, having renamed the node, I thought it made sense to keep the naming
consistent. Any further #define could be added to here instead of having to
add another file...
> > @@ -0,0 +1,187 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * R9A06G032 sysctrl IDs
> > + *
> > + * Copyright (C) 2018 Renesas Electronics Europe Limited
> > + *
> > + * Michel Pollet <michel.pollet@bp.renesas.com>, <buserror@gmail.com>
> > + * Derived from zx-reboot.c
> > + */
> > +
> > +#ifndef __DT_BINDINGS_R9A06G032_SYSCTRL_H__
> > +#define __DT_BINDINGS_R9A06G032_SYSCTRL_H__
> > +
> > +#define R9A06G032_CLKOUT 0
> > +#define R9A06G032_CLK_PLL_USB 1
> > +#define R9A06G032_CLK_48 1 /* AKA CLK_PLL_USB */
> > +#define R9A06G032_CLKOUT_D10 2
> > +#define R9A06G032_CLKOUT_D16 3
> > +#define R9A06G032_MSEBIS_CLK 3 /* AKA CLKOUT_D16 */
> > +#define R9A06G032_MSEBIM_CLK 3 /* AKA CLKOUT_D16 */
> > +#define R9A06G032_CLKOUT_D160 4
> > +#define R9A06G032_CLKOUT_D1OR2 5
> > +#define R9A06G032_CLK_DDRPHY_PLLCLK 5 /* AKA CLKOUT_D1OR2 */
> [...]
> I have 3 comments:
> 1. I had expected this list to match (both name- and order-wise)
Appendix
> C ("Clock Tree Structure") in the RZ/N1[DSL] User’s Manual: System
> Introduction, Multiplexing, Electrical and Mechanical Information.
> That would make it easier to review.
Well, that document was made a *long* time after the internal documentation
we used to generate the clock lists. There are a few things we had to do:
* Renumber peripherals. We decided a long time ago that u-boot and linux
would stick to zero based peripherals, and not one based numbers. It's next
to impossible to keep mixing number based across software base, so we use
UART0 while the hardware manual mentions UART1 -- It *is* documented
extensively with out SDK, and makes life using linux a lot easier. It's
across all our SDK, u-boot, webapps readmes, howtos etc.
* Rename some peripherals. Plenty of peripherals names made little sense
and had zero consistency, in fact, many names were different depending on
the place they were used! -- "flashnand"+"nand_flash"+"FNAND" etc,
"QUADSPI"+"QSPI" etc etc so we also re-normalized the names to match linux
conventions.
* Other internal reasons I can't document here
Also, the value here are made up anyway -- so I've decided to sort them to
make sure any clock that has a parent is numbered *after* the parent...
> 2. These definitions seems to be a mix of:
> 1. Internal core clocks,
> 2. Other core clocks,
> 3. Module clocks.
> The internal clocks do not need to be referenced from DT, so I would
> not make them part of the DT bindings.
Why? I'm told that "Bindings aren't for linux" -- why can't I imagine
'something' needing them? Why would I decide NOT to include them,
as they are there? I 'factored' a few of them to the same number when
applicable.
This is all done automatically BTW -- a script takes the original clocking
spreadsheet, and converts it into a table, correcting 'human input' as it
goes along.
> 3. It looks like the module clocks can be referred to by register offset
> and bit position, which is similar to how this is handled on R-Car
> SoCs.
> Perhaps you can just drop the definitions for these from the header
> file, and refer to them by (combined) register offset and bit
position
> instead?
> Or am I missing something?
> I believe this can also be done for the module resets (later).
If you look in the .c file, you'll see that most gate have not just one
register/bit pair associated with them -- there are several, spread
across registers.. Also, there are clocks in there with two gates,
or none. Given that, I've decided a separate numbering
made sense.
As mentioned, it's arbitrary, with 'root' clocks numbered lowered than
sub-clocks.
Thank you!
Michel
^ permalink raw reply
* Re: [PATCH v2] ARM: dts: imx51-zii-rdu1: Make sure SD1_WP is low
From: Nikita Yushchenko @ 2018-05-31 9:12 UTC (permalink / raw)
To: Andrey Smirnov, Shawn Guo
Cc: Fabio Estevam, Lucas Stach, Chris Healy, Rob Herring,
linux-arm-kernel, devicetree, linux-kernel
In-Reply-To: <20180526021238.30753-1-andrew.smirnov@gmail.com>
> Make sure that MX51_PAD_GPIO1_1 does not remain configure as
> ALT0/SD1_WP (it is out of reset). This is needed because of external
> pull-up resistor attached to that pad that, when left unchanged, will
> drive SD1_WP high preventing eSDHC1/eMMC from working correctly.
>
> To fix that add a pinmux configuration line configureing the pad to
> function as a GPIO. While we are at it, add a corresponding
> output-high GPIO hog in an effort to minimize current consumption.
>
> Cc: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Tested-By: Nikita Yushchenko <nikita.yoush@cogentembedded.com
Tested on 8.9'' RDU1. Without this patch, eMMC does not work, get
[ 40.801367] mmc0: Timeout waiting for hardware interrupt.
[ 40.806789] mmc0: sdhci: ============ SDHCI REGISTER DUMP ===========
... <dump follows>
With this patch, eMMC works correctly.
Nikita
^ permalink raw reply
* Re: [PATCH 06/15] drm/sun4i: tcon: Add support for tcon-top
From: Maxime Ripard @ 2018-05-31 9:21 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Jernej Škrabec, Rob Herring, Mark Rutland, dri-devel,
devicetree, linux-arm-kernel, linux-kernel, linux-clk,
linux-sunxi
In-Reply-To: <CAGb2v65whE-qW++cg+gu_o2O1dDdCWkumQB41nt3Aqa75Wp3dg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 5103 bytes --]
On Thu, May 24, 2018 at 03:01:09PM -0700, Chen-Yu Tsai wrote:
> >> > > + if (tcon->quirks->needs_tcon_top) {
> >> > > + struct device_node *np;
> >> > > +
> >> > > + np = of_parse_phandle(dev->of_node, "allwinner,tcon-top", 0);
> >> > > + if (np) {
> >> > > + struct platform_device *pdev;
> >> > > +
> >> > > + pdev = of_find_device_by_node(np);
> >> > > + if (pdev)
> >> > > + tcon->tcon_top = platform_get_drvdata(pdev);
> >> > > + of_node_put(np);
> >> > > +
> >> > > + if (!tcon->tcon_top)
> >> > > + return -EPROBE_DEFER;
> >> > > + }
> >> > > + }
> >> > > +
> >> >
> >> > I might have missed it, but I've not seen the bindings additions for
> >> > that property. This shouldn't really be done that way anyway, instead
> >> > of using a direct phandle, you should be using the of-graph, with the
> >> > TCON-top sitting where it belongs in the flow of data.
> >>
> >> Just to answer to the first question, it did describe it in "[PATCH 07/15] dt-
> >> bindings: display: sun4i-drm: Add R40 HDMI pipeline".
> >>
> >> As why I designed it that way - HW representation could be described that way
> >> (ASCII art makes sense when fixed width font is used to view it):
> >>
> >> / LCD0/LVDS0
> >> / TCON-LCD0
> >> | \ MIPI DSI
> >> mixer0 |
> >> \ / TCON-LCD1 - LCD1/LVDS1
> >> TCON-TOP
> >> / \ TCON-TV0 - TVE0/RGB
> >> mixer1 | \
> >> | TCON-TOP - HDMI
> >> | /
> >> \ TCON-TV1 - TVE1/RGB
> >>
> >> This is a bit simplified, since there is also TVE-TOP, which is responsible
> >> for sharing 4 DACs between both TVE encoders. You can have two TV outs (PAL/
> >> NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa. It even seems that you
> >> can arbitrarly choose which DAC is responsible for which signal, so there is a
> >> ton of possible end combinations, but I'm not 100% sure.
> >>
> >> Even though I wrote TCON-TOP twice, this is same unit in HW. R40 manual
> >> suggest more possibilities, although some of them seem wrong, like RGB feeding
> >> from LCD TCON. That is confirmed to be wrong when checking BSP code.
> >>
> >> Additionally, TCON-TOP comes in the middle of TVE0 and LCD0, TVE1 and LCD1 for
> >> pin muxing, although I'm not sure why is that needed at all, since according
> >> to R40 datasheet, TVE0 and TVE1 pins are dedicated and not on PORT D and PORT
> >> H, respectively, as TCON-TOP documentation suggest. However, HSYNC and PSYNC
> >> lines might be shared between TVE (when it works in RGB mode) and LCD. But
> >> that is just my guess since I'm not really familiar with RGB and LCD
> >> interfaces.
> >>
> >> I'm really not sure what would be the best representation in OF-graph. Can you
> >> suggest one?
> >
> > Rob might disagree on this one, but I don't see anything wrong with
> > having loops in the graph. If the TCON-TOP can be both the input and
> > output of the TCONs, then so be it, and have it described that way in
> > the graph.
> >
> > The code is already able to filter out nodes that have already been
> > added to the list of devices we need to wait for in the component
> > framework, so that should work as well.
> >
> > And we'd need to describe TVE-TOP as well, even though we don't have a
> > driver for it yet. That will simplify the backward compatibility later
> > on.
>
> I'm getting the feeling that TCON-TOP / TVE-TOP is the glue layer that
> binds everything together, and provides signal routing, kind of like
> DE-TOP on A64. So the signal mux controls that were originally found
> in TCON0 and TVE0 were moved out.
>
> The driver needs to know about that, but the graph about doesn't make
> much sense directly. Without looking at the manual, I understand it to
> likely be one mux between the mixers and TCONs, and one between the
> TCON-TVs and HDMI. Would it make more sense to just have the graph
> connections between the muxed components, and remove TCON-TOP from
> it, like we had in the past? A phandle could be used to reference
> the TCON-TOP for mux controls, in addition to the clocks and resets.
>
> For TVE, we would need something to represent each of the output pins,
> so the device tree can actually describe what kind of signal, be it
> each component of RGB/YUV or composite video, is wanted on each pin,
> if any. This is also needed on the A20 for the Cubietruck, so we can
> describe which pins are tied to the VGA connector, and which one does
> R, G, or B.
I guess we'll see how the DT maintainers feel about this, but my
impression is that the OF graph should model the flow of data between
the devices. If there's a mux somewhere, then the data is definitely
going through it, and as such it should be part of the graph.
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v7 1/5] dt-bindings: Add the r9a06g032-sysctrl.h file
From: Geert Uytterhoeven @ 2018-05-31 9:32 UTC (permalink / raw)
To: M P
Cc: Michel Pollet, Linux-Renesas, Simon Horman, Phil Edworthy,
Michel Pollet, Michael Turquette, Stephen Boyd, Rob Herring,
Mark Rutland, Geert Uytterhoeven, linux-clk,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List
In-Reply-To: <CAMMfpEwpiZnZEykzxQJMTwE+cDnOD_qA3nUzyB96DGTWPzSJrw@mail.gmail.com>
Hi Michel,
On Thu, May 31, 2018 at 11:11 AM, M P <buserror@gmail.com> wrote:
> On Fri, 25 May 2018 at 11:31, Geert Uytterhoeven <geert@linux-m68k.org>
> wrote:
>> On Thu, May 24, 2018 at 11:28 AM, Michel Pollet
>> <michel.pollet@bp.renesas.com> wrote:
>> > This adds the constants necessary to use the renesas,r9a06g032-sysctrl
> node.
>> > @@ -0,0 +1,187 @@
>> > +/* SPDX-License-Identifier: GPL-2.0 */
>> > +/*
>> > + * R9A06G032 sysctrl IDs
>> > + *
>> > + * Copyright (C) 2018 Renesas Electronics Europe Limited
>> > + *
>> > + * Michel Pollet <michel.pollet@bp.renesas.com>, <buserror@gmail.com>
>> > + * Derived from zx-reboot.c
>> > + */
>> > +
>> > +#ifndef __DT_BINDINGS_R9A06G032_SYSCTRL_H__
>> > +#define __DT_BINDINGS_R9A06G032_SYSCTRL_H__
>> > +
>> > +#define R9A06G032_CLKOUT 0
>> > +#define R9A06G032_CLK_PLL_USB 1
>> > +#define R9A06G032_CLK_48 1 /* AKA CLK_PLL_USB */
>> > +#define R9A06G032_CLKOUT_D10 2
>> > +#define R9A06G032_CLKOUT_D16 3
>> > +#define R9A06G032_MSEBIS_CLK 3 /* AKA CLKOUT_D16 */
>> > +#define R9A06G032_MSEBIM_CLK 3 /* AKA CLKOUT_D16 */
>> > +#define R9A06G032_CLKOUT_D160 4
>> > +#define R9A06G032_CLKOUT_D1OR2 5
>> > +#define R9A06G032_CLK_DDRPHY_PLLCLK 5 /* AKA CLKOUT_D1OR2 */
>
>> [...]
>
>> I have 3 comments:
>
>> 1. I had expected this list to match (both name- and order-wise)
> Appendix
>> C ("Clock Tree Structure") in the RZ/N1[DSL] User’s Manual: System
>> Introduction, Multiplexing, Electrical and Mechanical Information.
>> That would make it easier to review.
>
> Well, that document was made a *long* time after the internal documentation
> we used to generate the clock lists. There are a few things we had to do:
>
> * Renumber peripherals. We decided a long time ago that u-boot and linux
> would stick to zero based peripherals, and not one based numbers. It's next
> to impossible to keep mixing number based across software base, so we use
> UART0 while the hardware manual mentions UART1 -- It *is* documented
> extensively with out SDK, and makes life using linux a lot easier. It's
> across all our SDK, u-boot, webapps readmes, howtos etc.
>
> * Rename some peripherals. Plenty of peripherals names made little sense
> and had zero consistency, in fact, many names were different depending on
> the place they were used! -- "flashnand"+"nand_flash"+"FNAND" etc,
> "QUADSPI"+"QSPI" etc etc so we also re-normalized the names to match linux
> conventions.
>
> * Other internal reasons I can't document here
>
> Also, the value here are made up anyway -- so I've decided to sort them to
> make sure any clock that has a parent is numbered *after* the parent...
Well, all of that combines makes it very hard for us to review the list.
>> 2. These definitions seems to be a mix of:
>> 1. Internal core clocks,
>> 2. Other core clocks,
>> 3. Module clocks.
>
>> The internal clocks do not need to be referenced from DT, so I would
>> not make them part of the DT bindings.
>
> Why? I'm told that "Bindings aren't for linux" -- why can't I imagine
> 'something' needing them? Why would I decide NOT to include them,
> as they are there? I 'factored' a few of them to the same number when
Just general safety w.r.t. unchangeable DT definitions: anything that is
not listed here cannot be declared wrong later.
> applicable.
You're 100% sure they're the same?
> This is all done automatically BTW -- a script takes the original clocking
> spreadsheet, and converts it into a table, correcting 'human input' as it
> goes along.
So the internal spreadsheet doesn't match the public documentation neither?
>> 3. It looks like the module clocks can be referred to by register offset
>> and bit position, which is similar to how this is handled on R-Car
>> SoCs.
>> Perhaps you can just drop the definitions for these from the header
>> file, and refer to them by (combined) register offset and bit
> position
>> instead?
>> Or am I missing something?
>> I believe this can also be done for the module resets (later).
>
> If you look in the .c file, you'll see that most gate have not just one
> register/bit pair associated with them -- there are several, spread
> across registers.. Also, there are clocks in there with two gates,
> or none. Given that, I've decided a separate numbering
> made sense.
OK, fair enough.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 3/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
From: Stefan Agner @ 2018-05-31 9:37 UTC (permalink / raw)
To: boris.brezillon, dwmw2, computersforpeace, marek.vasut, robh+dt,
mark.rutland, thierry.reding, mturquette, sboyd
Cc: dev, miquel.raynal, richard, marcel, krzk, digetx,
benjamin.lindqvist, jonathanh, pdeschrijver, pgaikwad, mirza.krak,
linux-mtd, linux-tegra, devicetree, linux-kernel, linux-clk
In-Reply-To: <20180527215442.14760-4-stefan@agner.ch>
On 27.05.2018 23:54, Stefan Agner wrote:
> Add support for the NAND flash controller found on NVIDIA
> Tegra 2 SoCs. This implementation does not make use of the
> command queue feature. Regular operations/data transfers are
> done in PIO mode. Page read/writes with hardware ECC make
> use of the DMA for data transfer.
>
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
> MAINTAINERS | 7 +
> drivers/mtd/nand/raw/Kconfig | 6 +
> drivers/mtd/nand/raw/Makefile | 1 +
> drivers/mtd/nand/raw/tegra_nand.c | 999 ++++++++++++++++++++++++++++++
> 4 files changed, 1013 insertions(+)
> create mode 100644 drivers/mtd/nand/raw/tegra_nand.c
>
[...]
> +
> + chip->ecc.read_page = tegra_nand_read_page_hwecc;
> + chip->ecc.write_page = tegra_nand_write_page_hwecc;
> + /* Not functional for unknown reason...
> + chip->ecc.read_page_raw = tegra_nand_read_page;
> + chip->ecc.write_page_raw = tegra_nand_write_page;
> + */
I am giving up on these raw read/write_page functions. Using DMA without
HW ECC just seems not to work.
I do not get the CMD_DONE interrupt as I do when using DMA with HW ECC.
I tried with the same settings, just not enabling DMA (HW_ECC not set,
HW_ERR_CORRECTION not set, and also with/without PIPELINE_EN).
A register dump after a successful read with HW ECC looks like this:
[ 196.935199] CMD: 0x66890104
[ 196.938003] STATUS: 0x00000101
[ 196.941049] ISR: 0x00000000
[ 196.943865] CONFIG: 0x0084000b
[ 196.946914] DMA_MST_CTRL: 0x15000004
[ 196.950481] DMA_CFG_A: 0x00000fff
[ 196.954361] DMA_CFG_B: 0x00000000
[ 196.957670] FIFO: 0x0000aa00
Whereas without HW ECC completion times out (using
wait_for_completion_timeout already):
[ 226.128618] tegra-nand 70008000.nand: CMD timeout, last CMD:
0xe6890104
[ 226.135375] CMD: 0xe6890104
[ 226.138201] STATUS: 0x00000100
[ 226.141280] ISR: 0x00000100
[ 226.153372] CONFIG: 0x0084000b
[ 226.156423] DMA_MST_CTRL: 0x95000004
[ 226.159989] DMA_CFG_A: 0x00000fff
[ 226.164084] DMA_CFG_B: 0x00000000
[ 226.167393] FIFO: 0x0000aa00
It looks to me as if the DMA just does not start the data cycle. The
NAND seems to have read the page (RBSY0 is set).
Note that it is never explicitly stated whether DMA without HW ECC is
supported. There is some indication that it should: There are separated
bits to enable HW ECC/DMA, the reference manual states "If HW ECC is
enabled... " and a block diagram shows separate blocks for the ECC
Engine and NAND DMA control.
There is also indication that it does not: The chapter Restrictions
reads: "HW_ERR_CORRECTION = 0/1, doesn’t alter controller hardware
behavior. Software error correction scheme with HW_ERR_CORRECTION = 0,
is deprecated in Tegra 2 Processor."
Note that the default implementations nand_(read|write)_page_raw which
use exec_op do work fine! Unfortunately, the PIO mode only allows 4
bytes in a read cycle, hence raw read/write is slow...
--
Stefan
> + config = readl(ctrl->regs + CFG);
> + config |= CFG_PIPE_EN | CFG_SKIP_SPARE | CFG_SKIP_SPARE_SIZE_4;
> +
> + if (chip->options & NAND_BUSWIDTH_16)
> + config |= CFG_BUS_WIDTH_16;
> +
> + switch (chip->ecc.algo) {
> + case NAND_ECC_RS:
> + bits_per_step = BITS_PER_STEP_RS * chip->ecc.strength;
> + mtd_set_ooblayout(mtd, &tegra_nand_oob_rs_ops);
> + switch (chip->ecc.strength) {
> + case 4:
> + config |= CFG_ECC_SEL | CFG_TVAL_4;
> + break;
> + case 6:
> + config |= CFG_ECC_SEL | CFG_TVAL_6;
> + break;
> + case 8:
> + config |= CFG_ECC_SEL | CFG_TVAL_8;
> + break;
> + default:
> + dev_err(dev, "ECC strength %d not supported\n",
> + chip->ecc.strength);
> + return -EINVAL;
> + }
> + break;
> + case NAND_ECC_BCH:
> + bits_per_step = BITS_PER_STEP_BCH * chip->ecc.strength;
> + mtd_set_ooblayout(mtd, &tegra_nand_oob_bch_ops);
> + switch (chip->ecc.strength) {
> + case 4:
> + bch_config = BCH_TVAL_4;
> + break;
> + case 8:
> + bch_config = BCH_TVAL_8;
> + break;
> + case 14:
> + bch_config = BCH_TVAL_14;
> + break;
> + case 16:
> + bch_config = BCH_TVAL_16;
> + break;
> + default:
> + dev_err(dev, "ECC strength %d not supported\n",
> + chip->ecc.strength);
> + return -EINVAL;
> + }
> + break;
> + default:
> + dev_err(dev, "ECC algorithm not supported\n");
> + return -EINVAL;
> + }
> +
> + chip->ecc.bytes = DIV_ROUND_UP(bits_per_step, 8);
> +
> + switch (mtd->writesize) {
> + case 256:
> + config |= CFG_PS_256;
> + break;
> + case 512:
> + config |= CFG_PS_512;
> + break;
> + case 1024:
> + config |= CFG_PS_1024;
> + break;
> + case 2048:
> + config |= CFG_PS_2048;
> + break;
> + case 4096:
> + config |= CFG_PS_4096;
> + break;
> + default:
> + dev_err(dev, "unhandled writesize %d\n", mtd->writesize);
> + return -ENODEV;
> + }
> +
> + writel(config, ctrl->regs + CFG);
> + writel(bch_config, ctrl->regs + BCH_CONFIG);
> +
> + err = nand_scan_tail(mtd);
> + if (err)
> + return err;
> +
> + config |= CFG_TAG_BYTE_SIZE(mtd_ooblayout_count_freebytes(mtd) - 1);
> + writel(config, ctrl->regs + CFG);
> +
> + err = mtd_device_register(mtd, NULL, 0);
> + if (err)
> + return err;
> +
> + return 0;
> +}
> +
> +static int tegra_nand_probe(struct platform_device *pdev)
> +{
> + struct reset_control *rst;
> + struct tegra_nand_controller *ctrl;
> + struct resource *res;
> + unsigned long value;
> + int irq, err = 0;
> +
> + ctrl = devm_kzalloc(&pdev->dev, sizeof(*ctrl), GFP_KERNEL);
> + if (!ctrl)
> + return -ENOMEM;
> +
> + ctrl->dev = &pdev->dev;
> + nand_hw_control_init(&ctrl->controller);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + ctrl->regs = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(ctrl->regs))
> + return PTR_ERR(ctrl->regs);
> +
> + rst = devm_reset_control_get(&pdev->dev, "nand");
> + if (IS_ERR(rst))
> + return PTR_ERR(rst);
> +
> + ctrl->clk = devm_clk_get(&pdev->dev, "nand");
> + if (IS_ERR(ctrl->clk))
> + return PTR_ERR(ctrl->clk);
> +
> + err = clk_prepare_enable(ctrl->clk);
> + if (err)
> + return err;
> +
> + reset_control_reset(rst);
> +
> + value = HWSTATUS_RDSTATUS_MASK(1) | HWSTATUS_RDSTATUS_VALUE(0) |
> + HWSTATUS_RBSY_MASK(NAND_STATUS_READY) |
> + HWSTATUS_RBSY_VALUE(NAND_STATUS_READY);
> + writel(NAND_CMD_STATUS, ctrl->regs + HWSTATUS_CMD);
> + writel(value, ctrl->regs + HWSTATUS_MASK);
> +
> + init_completion(&ctrl->command_complete);
> + init_completion(&ctrl->dma_complete);
> +
> + /* clear interrupts */
> + value = readl(ctrl->regs + ISR);
> + writel(value, ctrl->regs + ISR);
> +
> + irq = platform_get_irq(pdev, 0);
> + err = devm_request_irq(&pdev->dev, irq, tegra_nand_irq, 0,
> + dev_name(&pdev->dev), ctrl);
> + if (err)
> + goto err_disable_clk;
> +
> + writel(DMA_CTRL_IS_DONE, ctrl->regs + DMA_CTRL);
> +
> + /* enable interrupts */
> + value = IER_UND | IER_OVR | IER_CMD_DONE | IER_ECC_ERR | IER_GIE;
> + writel(value, ctrl->regs + IER);
> +
> + /* reset config */
> + writel(0, ctrl->regs + CFG);
> +
> + err = tegra_nand_chips_init(ctrl->dev, ctrl);
> + if (err)
> + goto err_disable_clk;
> +
> + platform_set_drvdata(pdev, ctrl);
> +
> + return 0;
> +
> +err_disable_clk:
> + clk_disable_unprepare(ctrl->clk);
> + return err;
> +}
> +
> +static int tegra_nand_remove(struct platform_device *pdev)
> +{
> + struct tegra_nand_controller *ctrl = platform_get_drvdata(pdev);
> +
> + nand_release(nand_to_mtd(ctrl->chip));
> +
> + clk_disable_unprepare(ctrl->clk);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id tegra_nand_of_match[] = {
> + { .compatible = "nvidia,tegra20-nand" },
> + { /* sentinel */ }
> +};
> +
> +static struct platform_driver tegra_nand_driver = {
> + .driver = {
> + .name = "tegra-nand",
> + .of_match_table = tegra_nand_of_match,
> + },
> + .probe = tegra_nand_probe,
> + .remove = tegra_nand_remove,
> +};
> +module_platform_driver(tegra_nand_driver);
> +
> +MODULE_DESCRIPTION("NVIDIA Tegra NAND driver");
> +MODULE_AUTHOR("Thierry Reding <thierry.reding@nvidia.com>");
> +MODULE_AUTHOR("Lucas Stach <dev@lynxeye.de>");
> +MODULE_AUTHOR("Stefan Agner <stefan@agner.ch>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DEVICE_TABLE(of, tegra_nand_of_match);
^ permalink raw reply
* Re: [PATCH] arm64: allwinner: a64-amarula-relic: Enable AP6330 WiFi support
From: Maxime Ripard @ 2018-05-31 9:39 UTC (permalink / raw)
To: Jagan Teki
Cc: Chen-Yu Tsai, Michael Trimarchi, Icenowy Zheng,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20180529172238.9718-1-jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 331 bytes --]
Hi,
On Tue, May 29, 2018 at 10:52:38PM +0530, Jagan Teki wrote:
> +&rtc {
> + clock-output-names = "rtc-osc32k", "rtc-osc32k-out";
> + clocks = <&osc32k>;
> + #clock-cells = <1>;
> +};
It should be in the DTSI
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v2 0/8] gnss: add new GNSS subsystem
From: H. Nikolaus Schaller @ 2018-05-31 9:52 UTC (permalink / raw)
To: Johan Hovold
Cc: Richard Cochran, Greg Kroah-Hartman, Rob Herring, Mark Rutland,
Andreas Kemnade, Arnd Bergmann, Pavel Machek, Marcel Holtmann,
Sebastian Reichel, Tony Lindgren, linux-kernel, devicetree
In-Reply-To: <20180531085242.GE3259@localhost>
> Am 31.05.2018 um 10:52 schrieb Johan Hovold <johan@kernel.org>:
>
> On Wed, May 30, 2018 at 07:38:22AM -0700, Richard Cochran wrote:
>> On Wed, May 30, 2018 at 12:32:34PM +0200, Johan Hovold wrote:
>>> Another possible extension is to add generic 1PPS support.
>>
>> There are two possibilities to consider.
>>
>> 1. If the PPS causes an interrupt, then it should hook into the PPS
>> subsystem.
>
> Registering a PPS child device is what I had in mind for this.
This seems to be duplicating functionality that is already solved by
https://elixir.bootlin.com/linux/v4.17-rc7/source/Documentation/devicetree/bindings/pps/pps-gpio.txt
and
https://elixir.bootlin.com/linux/v4.17-rc7/source/drivers/pps/clients/pps-gpio.c
Or what is bad with just using that?
BR,
Nikolaus
^ permalink raw reply
* Re: [PATCH v2 3/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
From: Stefan Agner @ 2018-05-31 9:55 UTC (permalink / raw)
To: boris.brezillon, dwmw2, computersforpeace, marek.vasut, robh+dt,
mark.rutland, thierry.reding, mturquette, sboyd
Cc: dev, miquel.raynal, richard, marcel, krzk, digetx,
benjamin.lindqvist, jonathanh, pdeschrijver, pgaikwad, mirza.krak,
linux-mtd, linux-tegra, devicetree, linux-kernel, linux-clk
In-Reply-To: <80eb6a514e96cdbd460c6a0937a9dff9@agner.ch>
On 31.05.2018 11:37, Stefan Agner wrote:
> On 27.05.2018 23:54, Stefan Agner wrote:
>> Add support for the NAND flash controller found on NVIDIA
>> Tegra 2 SoCs. This implementation does not make use of the
>> command queue feature. Regular operations/data transfers are
>> done in PIO mode. Page read/writes with hardware ECC make
>> use of the DMA for data transfer.
>>
>> Signed-off-by: Lucas Stach <dev@lynxeye.de>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>> MAINTAINERS | 7 +
>> drivers/mtd/nand/raw/Kconfig | 6 +
>> drivers/mtd/nand/raw/Makefile | 1 +
>> drivers/mtd/nand/raw/tegra_nand.c | 999 ++++++++++++++++++++++++++++++
>> 4 files changed, 1013 insertions(+)
>> create mode 100644 drivers/mtd/nand/raw/tegra_nand.c
>>
> [...]
>> +
>> + chip->ecc.read_page = tegra_nand_read_page_hwecc;
>> + chip->ecc.write_page = tegra_nand_write_page_hwecc;
>> + /* Not functional for unknown reason...
>> + chip->ecc.read_page_raw = tegra_nand_read_page;
>> + chip->ecc.write_page_raw = tegra_nand_write_page;
>> + */
>
> I am giving up on these raw read/write_page functions. Using DMA without
> HW ECC just seems not to work.
>
> I do not get the CMD_DONE interrupt as I do when using DMA with HW ECC.
> I tried with the same settings, just not enabling DMA (HW_ECC not set,
> HW_ERR_CORRECTION not set, and also with/without PIPELINE_EN).
s/not enabling DMA/not enabling HW ECC/
>
> A register dump after a successful read with HW ECC looks like this:
> [ 196.935199] CMD: 0x66890104
> [ 196.938003] STATUS: 0x00000101
> [ 196.941049] ISR: 0x00000000
> [ 196.943865] CONFIG: 0x0084000b
> [ 196.946914] DMA_MST_CTRL: 0x15000004
> [ 196.950481] DMA_CFG_A: 0x00000fff
> [ 196.954361] DMA_CFG_B: 0x00000000
> [ 196.957670] FIFO: 0x0000aa00
>
> Whereas without HW ECC completion times out (using
> wait_for_completion_timeout already):
> [ 226.128618] tegra-nand 70008000.nand: CMD timeout, last CMD:
> 0xe6890104
> [ 226.135375] CMD: 0xe6890104
> [ 226.138201] STATUS: 0x00000100
> [ 226.141280] ISR: 0x00000100
> [ 226.153372] CONFIG: 0x0084000b
> [ 226.156423] DMA_MST_CTRL: 0x95000004
> [ 226.159989] DMA_CFG_A: 0x00000fff
> [ 226.164084] DMA_CFG_B: 0x00000000
> [ 226.167393] FIFO: 0x0000aa00
>
> It looks to me as if the DMA just does not start the data cycle. The
> NAND seems to have read the page (RBSY0 is set).
>
>
> Note that it is never explicitly stated whether DMA without HW ECC is
> supported. There is some indication that it should: There are separated
> bits to enable HW ECC/DMA, the reference manual states "If HW ECC is
> enabled... " and a block diagram shows separate blocks for the ECC
> Engine and NAND DMA control.
>
> There is also indication that it does not: The chapter Restrictions
> reads: "HW_ERR_CORRECTION = 0/1, doesn’t alter controller hardware
> behavior. Software error correction scheme with HW_ERR_CORRECTION = 0,
> is deprecated in Tegra 2 Processor."
>
> Note that the default implementations nand_(read|write)_page_raw which
> use exec_op do work fine! Unfortunately, the PIO mode only allows 4
> bytes in a read cycle, hence raw read/write is slow...
>
> --
> Stefan
>
>
>> + config = readl(ctrl->regs + CFG);
>> + config |= CFG_PIPE_EN | CFG_SKIP_SPARE | CFG_SKIP_SPARE_SIZE_4;
>> +
>> + if (chip->options & NAND_BUSWIDTH_16)
>> + config |= CFG_BUS_WIDTH_16;
>> +
>> + switch (chip->ecc.algo) {
>> + case NAND_ECC_RS:
>> + bits_per_step = BITS_PER_STEP_RS * chip->ecc.strength;
>> + mtd_set_ooblayout(mtd, &tegra_nand_oob_rs_ops);
>> + switch (chip->ecc.strength) {
>> + case 4:
>> + config |= CFG_ECC_SEL | CFG_TVAL_4;
>> + break;
>> + case 6:
>> + config |= CFG_ECC_SEL | CFG_TVAL_6;
>> + break;
>> + case 8:
>> + config |= CFG_ECC_SEL | CFG_TVAL_8;
>> + break;
>> + default:
>> + dev_err(dev, "ECC strength %d not supported\n",
>> + chip->ecc.strength);
>> + return -EINVAL;
>> + }
>> + break;
>> + case NAND_ECC_BCH:
>> + bits_per_step = BITS_PER_STEP_BCH * chip->ecc.strength;
>> + mtd_set_ooblayout(mtd, &tegra_nand_oob_bch_ops);
>> + switch (chip->ecc.strength) {
>> + case 4:
>> + bch_config = BCH_TVAL_4;
>> + break;
>> + case 8:
>> + bch_config = BCH_TVAL_8;
>> + break;
>> + case 14:
>> + bch_config = BCH_TVAL_14;
>> + break;
>> + case 16:
>> + bch_config = BCH_TVAL_16;
>> + break;
>> + default:
>> + dev_err(dev, "ECC strength %d not supported\n",
>> + chip->ecc.strength);
>> + return -EINVAL;
>> + }
>> + break;
>> + default:
>> + dev_err(dev, "ECC algorithm not supported\n");
>> + return -EINVAL;
>> + }
>> +
>> + chip->ecc.bytes = DIV_ROUND_UP(bits_per_step, 8);
>> +
>> + switch (mtd->writesize) {
>> + case 256:
>> + config |= CFG_PS_256;
>> + break;
>> + case 512:
>> + config |= CFG_PS_512;
>> + break;
>> + case 1024:
>> + config |= CFG_PS_1024;
>> + break;
>> + case 2048:
>> + config |= CFG_PS_2048;
>> + break;
>> + case 4096:
>> + config |= CFG_PS_4096;
>> + break;
>> + default:
>> + dev_err(dev, "unhandled writesize %d\n", mtd->writesize);
>> + return -ENODEV;
>> + }
>> +
>> + writel(config, ctrl->regs + CFG);
>> + writel(bch_config, ctrl->regs + BCH_CONFIG);
>> +
>> + err = nand_scan_tail(mtd);
>> + if (err)
>> + return err;
>> +
>> + config |= CFG_TAG_BYTE_SIZE(mtd_ooblayout_count_freebytes(mtd) - 1);
>> + writel(config, ctrl->regs + CFG);
>> +
>> + err = mtd_device_register(mtd, NULL, 0);
>> + if (err)
>> + return err;
>> +
>> + return 0;
>> +}
>> +
>> +static int tegra_nand_probe(struct platform_device *pdev)
>> +{
>> + struct reset_control *rst;
>> + struct tegra_nand_controller *ctrl;
>> + struct resource *res;
>> + unsigned long value;
>> + int irq, err = 0;
>> +
>> + ctrl = devm_kzalloc(&pdev->dev, sizeof(*ctrl), GFP_KERNEL);
>> + if (!ctrl)
>> + return -ENOMEM;
>> +
>> + ctrl->dev = &pdev->dev;
>> + nand_hw_control_init(&ctrl->controller);
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + ctrl->regs = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(ctrl->regs))
>> + return PTR_ERR(ctrl->regs);
>> +
>> + rst = devm_reset_control_get(&pdev->dev, "nand");
>> + if (IS_ERR(rst))
>> + return PTR_ERR(rst);
>> +
>> + ctrl->clk = devm_clk_get(&pdev->dev, "nand");
>> + if (IS_ERR(ctrl->clk))
>> + return PTR_ERR(ctrl->clk);
>> +
>> + err = clk_prepare_enable(ctrl->clk);
>> + if (err)
>> + return err;
>> +
>> + reset_control_reset(rst);
>> +
>> + value = HWSTATUS_RDSTATUS_MASK(1) | HWSTATUS_RDSTATUS_VALUE(0) |
>> + HWSTATUS_RBSY_MASK(NAND_STATUS_READY) |
>> + HWSTATUS_RBSY_VALUE(NAND_STATUS_READY);
>> + writel(NAND_CMD_STATUS, ctrl->regs + HWSTATUS_CMD);
>> + writel(value, ctrl->regs + HWSTATUS_MASK);
>> +
>> + init_completion(&ctrl->command_complete);
>> + init_completion(&ctrl->dma_complete);
>> +
>> + /* clear interrupts */
>> + value = readl(ctrl->regs + ISR);
>> + writel(value, ctrl->regs + ISR);
>> +
>> + irq = platform_get_irq(pdev, 0);
>> + err = devm_request_irq(&pdev->dev, irq, tegra_nand_irq, 0,
>> + dev_name(&pdev->dev), ctrl);
>> + if (err)
>> + goto err_disable_clk;
>> +
>> + writel(DMA_CTRL_IS_DONE, ctrl->regs + DMA_CTRL);
>> +
>> + /* enable interrupts */
>> + value = IER_UND | IER_OVR | IER_CMD_DONE | IER_ECC_ERR | IER_GIE;
>> + writel(value, ctrl->regs + IER);
>> +
>> + /* reset config */
>> + writel(0, ctrl->regs + CFG);
>> +
>> + err = tegra_nand_chips_init(ctrl->dev, ctrl);
>> + if (err)
>> + goto err_disable_clk;
>> +
>> + platform_set_drvdata(pdev, ctrl);
>> +
>> + return 0;
>> +
>> +err_disable_clk:
>> + clk_disable_unprepare(ctrl->clk);
>> + return err;
>> +}
>> +
>> +static int tegra_nand_remove(struct platform_device *pdev)
>> +{
>> + struct tegra_nand_controller *ctrl = platform_get_drvdata(pdev);
>> +
>> + nand_release(nand_to_mtd(ctrl->chip));
>> +
>> + clk_disable_unprepare(ctrl->clk);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id tegra_nand_of_match[] = {
>> + { .compatible = "nvidia,tegra20-nand" },
>> + { /* sentinel */ }
>> +};
>> +
>> +static struct platform_driver tegra_nand_driver = {
>> + .driver = {
>> + .name = "tegra-nand",
>> + .of_match_table = tegra_nand_of_match,
>> + },
>> + .probe = tegra_nand_probe,
>> + .remove = tegra_nand_remove,
>> +};
>> +module_platform_driver(tegra_nand_driver);
>> +
>> +MODULE_DESCRIPTION("NVIDIA Tegra NAND driver");
>> +MODULE_AUTHOR("Thierry Reding <thierry.reding@nvidia.com>");
>> +MODULE_AUTHOR("Lucas Stach <dev@lynxeye.de>");
>> +MODULE_AUTHOR("Stefan Agner <stefan@agner.ch>");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DEVICE_TABLE(of, tegra_nand_of_match);
^ 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