* [PATCH v11] clk: add MOXA ART SoCs clock driver
From: Jonas Jensen @ 2014-01-28 11:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390308261-4026-1-git-send-email-jonas.jensen@gmail.com>
MOXA ART SoCs allow to determine PLL output and APB frequencies
by reading registers holding multiplier and divisor information.
Add a clock driver for this SoC.
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
Thanks for the replies,
Changes since v10:
1. add clock-specifier to DT binding description
2. remove local variable "rate"
3. add local variable "parent_name"
4. use clk_register_fixed_factor() instead of clk_register_fixed_rate()
5. remove flag CLK_IS_ROOT
Applies to next-20140128
.../bindings/clock/moxa,moxart-clock.txt | 48 +++++++++++
drivers/clk/Makefile | 1 +
drivers/clk/clk-moxart.c | 97 ++++++++++++++++++++++
3 files changed, 146 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
create mode 100644 drivers/clk/clk-moxart.c
diff --git a/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt b/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
new file mode 100644
index 0000000..fedea84
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
@@ -0,0 +1,48 @@
+Device Tree Clock bindings for arch-moxart
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+MOXA ART SoCs allow to determine PLL output and APB frequencies
+by reading registers holding multiplier and divisor information.
+
+
+PLL:
+
+Required properties:
+- compatible : Must be "moxa,moxart-pll-clock"
+- #clock-cells : Should be 0
+- reg : Should contain registers location and length
+- clocks : Should contain phandle + clock-specifier for the parent clock
+
+Optional properties:
+- clock-output-names : Should contain clock name
+
+
+APB:
+
+Required properties:
+- compatible : Must be "moxa,moxart-apb-clock"
+- #clock-cells : Should be 0
+- reg : Should contain registers location and length
+- clocks : Should contain phandle + clock-specifier for the parent clock
+
+Optional properties:
+- clock-output-names : Should contain clock name
+
+
+For example:
+
+ clk_pll: clk_pll at 98100000 {
+ compatible = "moxa,moxart-pll-clock";
+ #clock-cells = <0>;
+ reg = <0x98100000 0x34>;
+ };
+
+ clk_apb: clk_apb at 98100000 {
+ compatible = "moxa,moxart-apb-clock";
+ #clock-cells = <0>;
+ reg = <0x98100000 0x34>;
+ clocks = <&clk_pll>;
+ };
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 0faf730..7940d0c 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-composite.o
# SoCs specific
obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.o
obj-$(CONFIG_ARCH_EFM32) += clk-efm32gg.o
+obj-$(CONFIG_ARCH_MOXART) += clk-moxart.o
obj-$(CONFIG_ARCH_NOMADIK) += clk-nomadik.o
obj-$(CONFIG_ARCH_HIGHBANK) += clk-highbank.o
obj-$(CONFIG_ARCH_HI3xxx) += hisilicon/
diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
new file mode 100644
index 0000000..30a3b69
--- /dev/null
+++ b/drivers/clk/clk-moxart.c
@@ -0,0 +1,97 @@
+/*
+ * MOXA ART SoCs clock driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/clkdev.h>
+
+void __init moxart_of_pll_clk_init(struct device_node *node)
+{
+ static void __iomem *base;
+ struct clk *clk, *ref_clk;
+ unsigned int mul;
+ const char *name = node->name;
+ const char *parent_name;
+
+ of_property_read_string(node, "clock-output-names", &name);
+ parent_name = of_clk_get_parent_name(node, 0);
+
+ base = of_iomap(node, 0);
+ if (!base) {
+ pr_err("%s: of_iomap failed\n", node->full_name);
+ return;
+ }
+
+ mul = readl(base + 0x30) >> 3 & 0x3f;
+ iounmap(base);
+
+ ref_clk = of_clk_get(node, 0);
+ if (IS_ERR(ref_clk)) {
+ pr_err("%s: of_clk_get failed\n", node->full_name);
+ return;
+ }
+
+ clk = clk_register_fixed_factor(NULL, name, parent_name, 0, mul, 1);
+ if (IS_ERR(clk)) {
+ pr_err("%s: failed to register clock\n", node->full_name);
+ return;
+ }
+
+ clk_register_clkdev(clk, NULL, name);
+ of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(moxart_pll_clock, "moxa,moxart-pll-clock",
+ moxart_of_pll_clk_init);
+
+void __init moxart_of_apb_clk_init(struct device_node *node)
+{
+ static void __iomem *base;
+ struct clk *clk, *pll_clk;
+ unsigned int div, val;
+ unsigned int div_idx[] = { 2, 3, 4, 6, 8};
+ const char *name = node->name;
+ const char *parent_name;
+
+ of_property_read_string(node, "clock-output-names", &name);
+ parent_name = of_clk_get_parent_name(node, 0);
+
+ base = of_iomap(node, 0);
+ if (!base) {
+ pr_err("%s: of_iomap failed\n", node->full_name);
+ return;
+ }
+
+ val = readl(base + 0xc) >> 4 & 0x7;
+ iounmap(base);
+
+ if (val > 4)
+ val = 0;
+ div = div_idx[val] * 2;
+
+ pll_clk = of_clk_get(node, 0);
+ if (IS_ERR(pll_clk)) {
+ pr_err("%s: of_clk_get failed\n", node->full_name);
+ return;
+ }
+
+ clk = clk_register_fixed_factor(NULL, name, parent_name, 0, 1, div);
+ if (IS_ERR(clk)) {
+ pr_err("%s: failed to register clock\n", node->full_name);
+ return;
+ }
+
+ clk_register_clkdev(clk, NULL, name);
+ of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(moxart_apb_clock, "moxa,moxart-apb-clock",
+ moxart_of_apb_clk_init);
--
1.8.2.1
^ permalink raw reply related
* [PATCH 2/9] ARM: dts: imx6sl: remove the use of pingrp macros
From: Sascha Hauer @ 2014-01-28 11:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5923680.sl3G3EgCsY@phil>
On Tue, Jan 28, 2014 at 11:17:22AM +0100, Heiko St?bner wrote:
> Hi Shawn,
>
> On Sunday, 26. January 2014 00:43:04 Shawn Guo wrote:
> > We created the pingrp macros in imx6sl-pingrp.h for purpose of less LOC
> > when same pin group is used by multiple boards. However, DT maintainers
> > take it as an abuse of DTC macro support. So let's get rid of it to
> > make the pins used by given device more intuitive.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > ---
> > arch/arm/boot/dts/imx6sl-evk.dts | 120 ++++++++++++++++++++++++++----
> > arch/arm/boot/dts/imx6sl-pingrp.h | 148
> > ------------------------------------- arch/arm/boot/dts/imx6sl.dtsi |
> > 1 -
> > 3 files changed, 107 insertions(+), 162 deletions(-)
> > delete mode 100644 arch/arm/boot/dts/imx6sl-pingrp.h
> >
> > diff --git a/arch/arm/boot/dts/imx6sl-evk.dts
> > b/arch/arm/boot/dts/imx6sl-evk.dts index f5e4513..8594d13 100644
> > --- a/arch/arm/boot/dts/imx6sl-evk.dts
> > +++ b/arch/arm/boot/dts/imx6sl-evk.dts
> > @@ -86,55 +86,149 @@
> > };
> >
> > pinctrl_ecspi1: ecspi1grp {
> > - fsl,pins = <MX6SL_ECSPI1_PINGRP1>;
> > + fsl,pins = <
> > + MX6SL_PAD_ECSPI1_MISO__ECSPI1_MISO 0x100b1
> > + MX6SL_PAD_ECSPI1_MOSI__ECSPI1_MOSI 0x100b1
> > + MX6SL_PAD_ECSPI1_SCLK__ECSPI1_SCLK 0x100b1
> > + >;
> > };
> >
> > pinctrl_fec: fecgrp {
> > - fsl,pins = <MX6SL_FEC_PINGRP1>;
> > + fsl,pins = <
> > + MX6SL_PAD_FEC_MDC__FEC_MDC 0x1b0b0
> > + MX6SL_PAD_FEC_MDIO__FEC_MDIO 0x1b0b0
> > + MX6SL_PAD_FEC_CRS_DV__FEC_RX_DV 0x1b0b0
> > + MX6SL_PAD_FEC_RXD0__FEC_RX_DATA0 0x1b0b0
> > + MX6SL_PAD_FEC_RXD1__FEC_RX_DATA1 0x1b0b0
> > + MX6SL_PAD_FEC_TX_EN__FEC_TX_EN 0x1b0b0
> > + MX6SL_PAD_FEC_TXD0__FEC_TX_DATA0 0x1b0b0
> > + MX6SL_PAD_FEC_TXD1__FEC_TX_DATA1 0x1b0b0
> > + MX6SL_PAD_FEC_REF_CLK__FEC_REF_OUT 0x4001b0a8
> > + >;
> > };
>
> [... and so on for the other groups ... ]
>
> I'm confused now :-) . Current linux-next [0] shows the pin-settings as part
> of imx6sl.dtsi - a way a lot of other architectures organize their pingroups
> too, with the board file only referencing the relevant pingroups from the
> predefined ones of the soc.
Current mainline has all groups under the iomux node which has the
effect that all possible groups are compiled into every dtb resulting in
very bloated dtbs. So Shawn changed it to what's currently in next, but
this hasn't been accepted by the dt maintainers. Now this series tries
to address the concerns of the dt maintainers by not using macros that
expand to other macros.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [PATCH v3 1/3] ARM: sun7i/sun6i: irqchip: Add irqchip driver for NMI controller
From: Hans de Goede @ 2014-01-28 11:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140128104044.GC3867@lukather>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi,
On 01/28/2014 11:40 AM, Maxime Ripard wrote:
Jumping in here to try and clarify things, or so I hope at least :)
> On Fri, Jan 17, 2014 at 09:54:55AM +0100, Carlo Caione wrote:
>>>> +/* + * Ack level interrupts right before unmask + * + * In case of level-triggered interrupt, IRQ line must be acked before it + * is unmasked or else a double-interrupt is triggered + */ + +static void sunxi_sc_nmi_ack_and_unmask(struct irq_data *d) +{ + struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); + struct irq_chip_type *ct = irq_data_get_chip_type(d); + u32 mask = d->mask; + + if (irqd_get_trigger_type(d) & IRQ_TYPE_LEVEL_MASK) + ct->chip.irq_ack(d); + + irq_gc_lock(gc); + irq_reg_writel(mask, gc->reg_base + ct->regs.mask); +
>>>> irq_gc_unlock(gc); +}
>>>
>>> Hmmm, handle_level_irq seems to be doing exactly that already. It first masks and acks the interrupts, and then unmask it, so we should be fine, don't we?
>>
>> We don't, or at least handle_level_irq doesn't work for all the cases.
>
> This is what I find weird :)
>
>> Let's say (i.e. this is the cubieboard2 case) that to the irqchip is connected to the IRQ line of a PMIC accessed by I2C. In this case we cannot mask/ack the interrupt on the originating device in the hard interrupt handler (in which handle_level_irq is) since we need to access the I2C bus in an non-interrupt context.
>
> We agree here.
>
>> ACKing the IRQ in handle_level_irq at this point is pretty much useless since we still have to ACK the IRQs on the originating device and this will be done in a IRQ thread started after the hard IRQ handler.
>
> Ok, so what you're saying is that you want to adress this case:
>
> handle_level_irq | device device | mask ack handler irq acked unmask | | | | | | v v v v v v
>
> NMI -> GIC: +--------+ +----------------- ---------------+ +-----+
>
> PMIC -> NMI: +-+ +-+ ------------+ +-----------+ +--------------------
>
> Right?
No, the IRQ from the PMIC is a level sensitive IRQ, so it would look like this:
> handle_level_irq | device device | mask ack handler irq acked unmask | | | | | | v v v v v v
>
> NMI -> GIC: +------------------------------+ ---------------+ +--
>
> PMIC -> NMI: +-------------------------+ ------------+ +----------
The PMIC irq line won't go low until an i2c write to its irq status
registers write-clears all status bits for which the corresponding
bit in the irq-mask register is set.
And the only reason the NMI -> GIC also goes low is because the unmask
operation writes a second ack to the NMI controller in the unmask
callback of the NMI controller driver.
Note that we cannot use edge triggered interrupts here because the PMIC
has the typical setup with multiple irq status bits driving a single
irq line, so the irq handler does read irq-status, handle stuff,
write-clear irq-status. And if any other irq-status bits get set
between the read and write-clear the PMIC -> NMI line will stay
high, as it should since there are more interrupts to handle.
> But in this case, you would have two events coming from your device (the two rising edges), so you'd expect two interrupts. And in the case of a level triggered interrupt, the device would keep the interrupt line active until it's unmasked, so we wouldn't end up with this either.
>
>> sunxi_sc_nmi_ack_and_unmask is therefore called (by irq_finalize_oneshot) after the IRQ thread has been executed. After the IRQ thread has ACKed the IRQs on the originating device we can finally ACK and unmask again the NMI.
>
> And what happens if you get a new interrupt right between the end of the handler and the unmask?
The implicit ack done by the unmask will be ignored if the NMI line is still
high, just like the initial ack is ignored (which is why we need the mask),
and when the unmask completes the irq will immediately retrigger, as it should.
<snip>
>>> I really wonder whether it makes sense to have a generic chip here. It seems to be much more complicated than it should. It's only about a single interrupt interrupt chip here.
>>
>> I agree but is there any other way to manage the NMI without the driver of the device connected to NMI having to worry about acking/masking/unmasking/ etc..?
>
> Yes, just declare a "raw" irqchip. Pretty much like we do on irq-sun4i for example.
Hmm, I'm not going to say that using a raw irqchip here is a bad idea, it
sounds like it would be better.
But I don't think this will solve the need the mask / umask. The problem is
we need to do an i2c write to clear the interrupt source, which needs to
be done from a thread / workqueue. So when the interrupt handler finishes
the source won't be cleared yet, and AFAIK then only way to deal with this
is to mask the interrupt until we've cleared the interrupt source.
I agree that ideally we would be able to hide all this inside the NMI
controller driver, but I'm not sure if we can.
Regards,
Hans
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlLnjj8ACgkQF3VEtJrzE/skqACgjGLU2QLQUq9o0pU1DuuWIUpx
YngAoJmbmCGEhkRBy5xFmKuXapilqzmM
=BoL/
-----END PGP SIGNATURE-----
^ permalink raw reply
* [alsa-devel] [PATCH 4/4] ASoC: tda998x: adjust the audio hw parameters from EDID
From: Mark Brown @ 2014-01-28 11:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <s5hppncihn6.wl%tiwai@suse.de>
On Tue, Jan 28, 2014 at 10:23:57AM +0100, Takashi Iwai wrote:
> Mark Brown wrote:
> > On Mon, Jan 27, 2014 at 08:49:15PM +0000, Russell King - ARM Linux wrote:
> > > Yes, preferably as a generic ALSA helper rather than an ASoC helper -
> > > I don't see any need for this to be ASoC specific (I have a pure ALSA
> > > driver which has very similar code in it.)
> > Indeed, definitely ALSA generic - ideally we could factor a lot of the
> > integration with the video side out.
> Yes, indeed.
> OTOH, as discussed recently, we're heading to move from ELD parsing to
> more direct communication between video and audio drivers for
> HD-audio. ELD will be still provided to user-space, but not evaluated
> any longer in the new scenario.
That sort of refactoring being one of the best reasons to keep things
out of individual drivers! Having said all this I don't know if it's
worth blocking Jean-Francois' work on that, it's an improvement in
itself. Splitting the code out a bit would be good to help prepare but
having the full refactoring done might be too much of a blocker.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140128/6ec7cfaf/attachment.sig>
^ permalink raw reply
* [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group notifier block
From: Varun Sethi @ 2014-01-28 11:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140123195718.GC26399@alberich>
> -----Original Message-----
> From: iommu-bounces at lists.linux-foundation.org [mailto:iommu-
> bounces at lists.linux-foundation.org] On Behalf Of Andreas Herrmann
> Sent: Friday, January 24, 2014 1:27 AM
> To: Sethi Varun-B16395
> Cc: Andreas Herrmann; iommu at lists.linux-foundation.org; Will Deacon;
> linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group
> notifier block
>
> On Wed, Jan 22, 2014 at 07:07:40PM +0000, Varun Sethi wrote:
> > > -----Original Message-----
> > > From: Will Deacon [mailto:will.deacon at arm.com]
> > > Sent: Wednesday, January 22, 2014 9:04 PM
> > > To: Sethi Varun-B16395
> > > Cc: Andreas Herrmann; iommu at lists.linux-foundation.org; linux-arm-
> > > kernel at lists.infradead.org; Andreas Herrmann
> > > Subject: Re: [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group
> > > notifier block
> > >
> > > On Wed, Jan 22, 2014 at 01:54:11PM +0000, Varun Sethi wrote:
> > > > > > > Ok, so are you suggesting that we perform the isolation
> > > > > > > mapping in arm_smmu_add_device and drop the notifier
> altogether?
> > > > > > I think that should be fine, until we want to delay mapping
> > > > > > creation till driver bind time.
> > > > >
> > > > > Is there a hard dependency on that?
> > > > >
> > > > Not sure, may be Andreas can answer that.
> > >
> > > Ok. Andreas? I would have thought doing this *earlier* shouldn't be
> > > a problem (the DMA ops must be swizzled before the driver is probed).
> > >
> > > > > > But the "isolate device" property should dictate iommu group
> > > creation.
> > > > >
> > > > > The reason we added automatic group creation (per-device) is for
> > > > > VFIO, which expects all devices to be in a group regardless of
> > > > > the device isolation configuration.
> > > > >
> > > > IIUC, with the "isolate devices" property we ensure that there
> > > > would be independent SMR and S2CR per device. Is that correct?
> > >
> > > Yes, as part of giving them independent sets of page tables.
> > > Initially these tables don't have any valid mappings, but the
> > > dma-mapping API will populate them in response to
> dma_map_*/dma_alloc/etc.
> > >
> > > Not sure what this has to do with us putting devices into their own
> > > groups...
>
> > [Sethi Varun-B16395] Devices in an iommu group would share the dma
> > mapping, so shouldn't they be sharing the SMR and context registers?
>
> I aggree with the context but SMRs won't be shared. I think you just can
> say that a certain subset of the available SMRs will be used to map all
> devices in an iommu group to the same context. Depending on what
> streamIDs each device has you might have to use separate SMRs for each
> device to map it to the same context.
[Sethi Varun-B16395] IIUC the SMRs would unique per device, but there is a possibility where the number of context registers are restricted?
In that case IOMMU groups should correspond to unique stream IDs (and corresponding SMRs).
>
> > In case of the "isolate devices" property, each device would have its
> > own SMR and context registers, thus allowing the devices to have
> > independent mappings and allowing them to fall in separate iommu
> > groups.
>
> I aggree with Varun's view here. (Now that I take iommu groups into
> consideration.)
>
> But my goal with the "isolate devices" thing was twofold:
>
> 1. actual make use of SMMU for address translation for all master
> devices (instead of just bypassing the SMMU)
[Sethi Varun-B16395] even if masters have to share translation? But from the patch it seemed that we are creating mapping only if we find the isolate devices property.
>
> plus
>
> 2. put each master into it's own domain to isolate it
>
> The latter matches usage of separate iommu groups for devices. If we now
> use the isolate devices property to just control whether master devices
> fall into the same or separate iommu groups it seems to me we would need
> to have another mechanism that triggers the creation of a mapping for a
> group.
>
> What do you think?
>
[Sethi Varun-B16395] As mentioned before, we should be having iommu group per device (having a unique stream id). Isolate devices property just ensures that each device has a unique context. Now, for the devices for which we don't have the isolate device property, can't we have the multiple devices point to a common mapping.
-Varun
^ permalink raw reply
* [PATCH 1/3] mmc: add support for power-on sequencing through DT
From: Arnd Bergmann @ 2014-01-28 10:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFpOhQtiyCmzie3=sfg42Vv5_rReRNgUf8kALoE6TBe2cw@mail.gmail.com>
On Tuesday 28 January 2014, Ulf Hansson wrote:
> On 28 January 2014 01:59, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> > On 27.01.2014 11:19, Ulf Hansson wrote:
> >> There is already a host capability that I think we could use to handle
> >> this. MMC_CAP_NONREMOVABLE, the corresponding DT binding string is
> >> "non-removable", and it may be set per host device.
> >>
> >> Using this cap means the mmc_rescan process that runs to detect new
> >> cards, will only be executed once and during boot. So, we need to make
> >> sure all resources and powers are provided to the card at this point.
> >> Otherwise the card will not be detected.
> >
> > I don't quite like this requirement, especially if you consider
> > multi-platform kernels where a lot of drivers is going to be provided as
> > modules. WLAN drivers are especially good candidates. This means that even
> > if the card is powered off at boot-up, if user (or init system) loads
> > appropriate module, which powers the chip on, MMC core must be able to
> > notice this.
>
> To be able to detect the card, the WLAN driver doesn't have to be
> probed, only the "power controller" driver. I suppose this is were it
> becomes a bit tricky.
>
> Somehow the mmc core needs to be involved in the probe process of the
> power controller driver. Could perhaps the power controller bus be
> located in the mmc core and thus the power controller driver needs to
> register itself by using a new API from the mmc core? Similar how SDIO
> func driver's register themselves.
I think there is another option, which does have its own pros and cons:
We could move all the power handling back into the sdio function driver
if we allow a secondary detection path using DT rather than the probing
of the SDIO bus. Essentially you'd have to list the class/vendor/device
ID for each function that cannot be autodetected in DT, and have the
SDIO core pretend that it found the device just by looking at the
device nodes, and register the struct sdio_func so it can be bound to
the driver. The driver then does all the power handling in a device
specific way. At some point the hardware gets registered at the
mmc host, and the sdio core connects the bus state to the already present
sdio_func, possibly notifying the function driver that it has become
usable.
Obviously, this can only work for CAP_NONREMOVABLE devices, but those
are exactly the ones we are worried about here. The advantage is that
the power sequencing for a particular device can then be in device
specific code and can have an arbitrarily complex in the driver without
needing the mmc code to handle all possible corner cases.
Arnd
^ permalink raw reply
* [PATCH] clk: divider: fix rate calculation for fractional rates
From: Tomi Valkeinen @ 2014-01-28 10:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140128103253.GD15937@n2100.arm.linux.org.uk>
On 2014-01-28 12:32, Russell King - ARM Linux wrote:
>> Why I'm asking this is that for me (and probably for others also if
>> you've seen it used in the kernel code) it feels natural to have code like:
>>
>> rate = clk_round_rate(clk, rate);
>>
>> /* Verify the rounded rate here to see it's ok for the IP etc */
>>
>> /* The rate is ok, so set it */
>> clk_set_rate(clk, rate);
>
> If you want to do something with the rounded rate, then that's fine,
> you have a reason to do it this way. However, what I was referring to
> are drivers which literally do this:
>
> clk_set_rate(clk, clk_round_rate(clk, rate));
Thanks for clarification. Agreed, that's pointless. I gave the sequence
in the patch description just as an example for the sake of discussion
about the bug.
I didn't realize people actually do that in real code =).
Tomi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140128/97c9c93c/attachment.sig>
^ permalink raw reply
* [PATCH v3 1/3] ARM: sun7i/sun6i: irqchip: Add irqchip driver for NMI controller
From: Maxime Ripard @ 2014-01-28 10:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOQ7t2ay6UM-YAUSypRzC=_oSY=0L5RhD6FziG96j4aoK6RHsQ@mail.gmail.com>
On Fri, Jan 17, 2014 at 09:54:55AM +0100, Carlo Caione wrote:
> >> +/*
> >> + * Ack level interrupts right before unmask
> >> + *
> >> + * In case of level-triggered interrupt, IRQ line must be acked before it
> >> + * is unmasked or else a double-interrupt is triggered
> >> + */
> >> +
> >> +static void sunxi_sc_nmi_ack_and_unmask(struct irq_data *d)
> >> +{
> >> + struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> >> + struct irq_chip_type *ct = irq_data_get_chip_type(d);
> >> + u32 mask = d->mask;
> >> +
> >> + if (irqd_get_trigger_type(d) & IRQ_TYPE_LEVEL_MASK)
> >> + ct->chip.irq_ack(d);
> >> +
> >> + irq_gc_lock(gc);
> >> + irq_reg_writel(mask, gc->reg_base + ct->regs.mask);
> >> + irq_gc_unlock(gc);
> >> +}
> >
> > Hmmm, handle_level_irq seems to be doing exactly that already. It
> > first masks and acks the interrupts, and then unmask it, so we should
> > be fine, don't we?
>
> We don't, or at least handle_level_irq doesn't work for all the cases.
This is what I find weird :)
> Let's say (i.e. this is the cubieboard2 case) that to the irqchip is
> connected to the IRQ line of a PMIC accessed by I2C. In this case we
> cannot mask/ack the interrupt on the originating device in the hard
> interrupt handler (in which handle_level_irq is) since we need to
> access the I2C bus in an non-interrupt context.
We agree here.
> ACKing the IRQ in handle_level_irq at this point is pretty much
> useless since we still have to ACK the IRQs on the originating
> device and this will be done in a IRQ thread started after the hard
> IRQ handler.
Ok, so what you're saying is that you want to adress this case:
handle_level_irq
| device device
| mask ack handler irq acked unmask
| | | | | |
v v v v v v
NMI -> GIC:
+--------+ +-----------------
---------------+ +-----+
PMIC -> NMI:
+-+ +-+
------------+ +-----------+ +--------------------
Right?
But in this case, you would have two events coming from your device
(the two rising edges), so you'd expect two interrupts. And in the
case of a level triggered interrupt, the device would keep the
interrupt line active until it's unmasked, so we wouldn't end up with
this either.
> sunxi_sc_nmi_ack_and_unmask is therefore called (by
> irq_finalize_oneshot) after the IRQ thread has been executed. After
> the IRQ thread has ACKed the IRQs on the originating device we can
> finally ACK and unmask again the NMI.
And what happens if you get a new interrupt right between the end of
the handler and the unmask?
>
> >> +static inline void sunxi_sc_nmi_write(struct irq_chip_generic *gc, u32 off,
> >> + u32 val)
> >> +{
> >> + irq_reg_writel(val, gc->reg_base + off);
> >> +}
> >> +
> >> +static inline u32 sunxi_sc_nmi_read(struct irq_chip_generic *gc, u32 off)
> >> +{
> >> + return irq_reg_readl(gc->reg_base + off);
> >> +}
> >> +
> >> +static void sunxi_sc_nmi_handle_irq(unsigned int irq, struct irq_desc *desc)
> >> +{
> >> + struct irq_domain *domain = irq_desc_get_handler_data(desc);
> >> + struct irq_chip *chip = irq_get_chip(irq);
> >> + unsigned int virq = irq_find_mapping(domain, 0);
> >> +
> >> + chained_irq_enter(chip, desc);
> >> + generic_handle_irq(virq);
> >> + chained_irq_exit(chip, desc);
> >> +}
> >> +
> >> +static int sunxi_sc_nmi_set_type(struct irq_data *data, unsigned int flow_type)
> >> +{
> >> + struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
> >> + struct irq_chip_type *ct = gc->chip_types;
> >> + u32 src_type_reg;
> >> + u32 ctrl_off = ct->regs.type;
> >> + unsigned int src_type;
> >> + unsigned int i;
> >> +
> >> + irq_gc_lock(gc);
> >> +
> >> + switch (flow_type & IRQF_TRIGGER_MASK) {
> >> + case IRQ_TYPE_EDGE_FALLING:
> >> + src_type = SUNXI_SRC_TYPE_EDGE_FALLING;
> >> + break;
> >> + case IRQ_TYPE_EDGE_RISING:
> >> + src_type = SUNXI_SRC_TYPE_EDGE_RISING;
> >> + break;
> >> + case IRQ_TYPE_LEVEL_HIGH:
> >> + src_type = SUNXI_SRC_TYPE_LEVEL_HIGH;
> >> + break;
> >> + case IRQ_TYPE_NONE:
> >> + case IRQ_TYPE_LEVEL_LOW:
> >> + src_type = SUNXI_SRC_TYPE_LEVEL_LOW;
> >> + break;
> >> + default:
> >> + irq_gc_unlock(gc);
> >> + pr_err("%s: Cannot assign multiple trigger modes to IRQ %d.\n",
> >> + __func__, data->irq);
> >> + return -EBADR;
> >> + }
> >> +
> >> + irqd_set_trigger_type(data, flow_type);
> >> + irq_setup_alt_chip(data, flow_type);
> >> +
> >> + for (i = 0; i <= gc->num_ct; i++, ct++)
> >> + if (ct->type & flow_type)
> >> + ctrl_off = ct->regs.type;
> >> +
> >> + src_type_reg = sunxi_sc_nmi_read(gc, ctrl_off);
> >> + src_type_reg &= ~SUNXI_NMI_SRC_TYPE_MASK;
> >> + src_type_reg |= src_type;
> >> + sunxi_sc_nmi_write(gc, ctrl_off, src_type_reg);
> >> +
> >> + irq_gc_unlock(gc);
> >> +
> >> + return IRQ_SET_MASK_OK;
> >> +}
> >> +
> >> +static int __init sunxi_sc_nmi_irq_init(struct device_node *node,
> >> + struct sunxi_sc_nmi_reg_offs *reg_offs)
> >> +{
> >> + struct irq_domain *domain;
> >> + struct irq_chip_generic *gc;
> >> + unsigned int irq;
> >> + unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
> >> + int ret;
> >> +
> >> +
> >> + domain = irq_domain_add_linear(node, 1, &irq_generic_chip_ops, NULL);
> >> + if (!domain) {
> >> + pr_err("%s: Could not register interrupt domain.\n", node->name);
> >> + return -ENOMEM;
> >> + }
> >> +
> >> + ret = irq_alloc_domain_generic_chips(domain, 1, 2, node->name,
> >> + handle_level_irq, clr, 0,
> >> + IRQ_GC_INIT_MASK_CACHE);
> >> + if (ret) {
> >> + pr_err("%s: Could not allocate generic interrupt chip.\n",
> >> + node->name);
> >> + goto fail_irqd_remove;
> >> + }
> >> +
> >> + irq = irq_of_parse_and_map(node, 0);
> >> + if (irq <= 0) {
> >> + pr_err("%s: unable to parse irq\n", node->name);
> >> + ret = -EINVAL;
> >> + goto fail_irqd_remove;
> >> + }
> >> +
> >> + gc = irq_get_domain_generic_chip(domain, 0);
> >> + gc->reg_base = of_iomap(node, 0);
> >> + if (!gc->reg_base) {
> >> + pr_err("%s: unable to map resource\n", node->name);
> >> + ret = -ENOMEM;
> >> + goto fail_irqd_remove;
> >> + }
> >> +
> >> + gc->chip_types[0].type = IRQ_TYPE_LEVEL_MASK;
> >> + gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit;
> >> + gc->chip_types[0].chip.irq_mask = irq_gc_mask_clr_bit;
> >> + gc->chip_types[0].chip.irq_unmask = sunxi_sc_nmi_ack_and_unmask;
> >> + gc->chip_types[0].chip.irq_set_type = sunxi_sc_nmi_set_type;
> >> + gc->chip_types[0].regs.ack = reg_offs->pend;
> >> + gc->chip_types[0].regs.mask = reg_offs->enable;
> >> + gc->chip_types[0].regs.type = reg_offs->ctrl;
> >> +
> >> + gc->chip_types[1].type = IRQ_TYPE_EDGE_BOTH;
> >> + gc->chip_types[1].chip.name = gc->chip_types[0].chip.name;
> >> + gc->chip_types[1].chip.irq_ack = irq_gc_ack_set_bit;
> >> + gc->chip_types[1].chip.irq_mask = irq_gc_mask_clr_bit;
> >> + gc->chip_types[1].chip.irq_unmask = sunxi_sc_nmi_ack_and_unmask;
> >> + gc->chip_types[1].chip.irq_set_type = sunxi_sc_nmi_set_type;
> >> + gc->chip_types[1].regs.ack = reg_offs->pend;
> >> + gc->chip_types[1].regs.mask = reg_offs->enable;
> >> + gc->chip_types[1].regs.type = reg_offs->ctrl;
> >> + gc->chip_types[1].handler = handle_edge_irq;
> >> +
> >> + irq_set_handler_data(irq, domain);
> >> + irq_set_chained_handler(irq, sunxi_sc_nmi_handle_irq);
> >> +
> >> + sunxi_sc_nmi_write(gc, reg_offs->enable, 0);
> >> + sunxi_sc_nmi_write(gc, reg_offs->pend, 0x1);
> >
> > I really wonder whether it makes sense to have a generic chip here. It
> > seems to be much more complicated than it should. It's only about a
> > single interrupt interrupt chip here.
>
> I agree but is there any other way to manage the NMI without the
> driver of the device connected to NMI having to worry about
> acking/masking/unmasking/ etc..?
Yes, just declare a "raw" irqchip. Pretty much like we do on irq-sun4i
for example.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140128/416961c6/attachment.sig>
^ permalink raw reply
* [PATCH] clk: divider: fix rate calculation for fractional rates
From: Russell King - ARM Linux @ 2014-01-28 10:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E76E3A.8030807@ti.com>
On Tue, Jan 28, 2014 at 10:45:46AM +0200, Tomi Valkeinen wrote:
> Russell, I'd like to understand why you think the original example is bad:
>
> rate = clk_round_rate(clk, rate);
> clk_set_rate(clk, rate);
It's needlessly wasteful. All the processing for setting the rate is
repeated.
> If the definition of clk_round_rate is basically "clk_set_rate without
> actually setting the rate", I agree that the above code is not good as
> it might not work correctly.
>
> However, if the following code you gave should work:
>
> rate = clk_get_rate(clk);
> clk_set_rate(clk, rate);
> assert(clk_get_rate(clk) == rate);
>
> then the original example should also always work, as it's almost the
> same as:
>
> /* this is the "round" part */
> clk_set_rate(clk, rate);
> rate = clk_get_rate(clk);
>
> clk_set_rate(clk, rate);
> assert(clk_get_rate(clk) == rate);
Okay, now ask yourself this - would you code the above into your driver
with no processing between the two? It seems that some people would!
> Why I'm asking this is that for me (and probably for others also if
> you've seen it used in the kernel code) it feels natural to have code like:
>
> rate = clk_round_rate(clk, rate);
>
> /* Verify the rounded rate here to see it's ok for the IP etc */
>
> /* The rate is ok, so set it */
> clk_set_rate(clk, rate);
If you want to do something with the rounded rate, then that's fine,
you have a reason to do it this way. However, what I was referring to
are drivers which literally do this:
clk_set_rate(clk, clk_round_rate(clk, rate));
In other words, they think that they must always round the rate before
passing it into clk_set_rate() even though they make no other use of
the rounded rate. That is completely wasteful and unnecessary. It
might as well have clk_round_rate() replaced by a udelay() to waste
some CPU cycles just for the hell of it.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply
* [PATCH v5 01/20] ARM: Introduce atomic MMIO modify
From: Ezequiel Garcia @ 2014-01-28 10:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390836440-12744-2-git-send-email-ezequiel.garcia@free-electrons.com>
On Mon, Jan 27, 2014 at 12:27:01PM -0300, Ezequiel Garcia wrote:
> Some SoC have MMIO regions that are shared across orthogonal
> subsystems. This commit implements a possible solution for the
> thread-safe access of such regions through a spinlock-protected API.
>
> Concurrent access is protected with a single spinlock for the
> entire MMIO address space. While this protects shared-registers,
> it also serializes access to unrelated/unshared registers.
>
> We add relaxed and non-relaxed variants, by using writel_relaxed and writel,
> respectively. The rationale for this is that some users may not require
> register write completion but only thread-safe access to a register.
>
> Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>
> Tested-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
> Russell,
>
> Can you confirm this patch is on its way to v3.14?
>
ping?
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH 1/9] ARM: dts: imx6qdl: remove the use of pingrp macros
From: Shawn Guo @ 2014-01-28 10:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140127143745.GM15937@n2100.arm.linux.org.uk>
On Mon, Jan 27, 2014 at 02:37:45PM +0000, Russell King - ARM Linux wrote:
> On Sun, Jan 26, 2014 at 12:43:03AM +0800, Shawn Guo wrote:
> > arch/arm/boot/dts/imx6dl-hummingboard.dts | 5 +-
> > arch/arm/boot/dts/imx6qdl-microsom.dtsi | 5 +-
>
> I've merged your changes here into my local copy of these just to reduce
> the conflicts - unfortunately, it's taken soo long to deal with the above
> that the cubox-i has now been released, which has prompted some
> reorganisation between the above two files.
>
> I would much rather you dropped these two entirely, and let me push them
> upstream, rather than having some nasty conflicts which result from this.
Dropped hummingboard from my tree.
Shawn
^ permalink raw reply
* [PATCH v5 14/20] watchdog: orion: Add support for Armada 370 and Armada XP SoC
From: Ezequiel Garcia @ 2014-01-28 10:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140127173624.GT15937@n2100.arm.linux.org.uk>
On Mon, Jan 27, 2014 at 05:36:24PM +0000, Russell King - ARM Linux wrote:
[..]
> > +static int armadaxp_wdt_clock_init(struct platform_device *pdev,
> > + struct orion_watchdog *dev)
> > +{
> > + int ret;
> > +
> > + dev->clk = of_clk_get_by_name(pdev->dev.of_node, "fixed");
> > + if (IS_ERR(dev->clk))
> > + return PTR_ERR(dev->clk);
> > + ret = clk_prepare_enable(dev->clk);
> > + if (ret)
> > + return ret;
> > +
> > + /* Enable the fixed watchdog clock input */
> > + atomic_io_modify(dev->reg + TIMER_CTRL,
> > + WDT_AXP_FIXED_ENABLE_BIT,
> > + WDT_AXP_FIXED_ENABLE_BIT);
> > +
> > + dev->clk_rate = clk_get_rate(dev->clk);
> > + return 0;
> > +}
>
> Doesn't this result in dev->clk being leaked? Or at least a difference
> in the way dev->clk needs to be cleaned up between these two functions?
>
Yes, indeed.
> I think it would be better in this case to use the standard clk_get() in
> the first function and always use clk_put()... until there is a devm_*
> version of the of_clk_get* functions.
>
Sound good.
Thanks,
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH 2/9] ARM: dts: imx6sl: remove the use of pingrp macros
From: Heiko Stübner @ 2014-01-28 10:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390668191-20289-3-git-send-email-shawn.guo@linaro.org>
Hi Shawn,
On Sunday, 26. January 2014 00:43:04 Shawn Guo wrote:
> We created the pingrp macros in imx6sl-pingrp.h for purpose of less LOC
> when same pin group is used by multiple boards. However, DT maintainers
> take it as an abuse of DTC macro support. So let's get rid of it to
> make the pins used by given device more intuitive.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> arch/arm/boot/dts/imx6sl-evk.dts | 120 ++++++++++++++++++++++++++----
> arch/arm/boot/dts/imx6sl-pingrp.h | 148
> ------------------------------------- arch/arm/boot/dts/imx6sl.dtsi |
> 1 -
> 3 files changed, 107 insertions(+), 162 deletions(-)
> delete mode 100644 arch/arm/boot/dts/imx6sl-pingrp.h
>
> diff --git a/arch/arm/boot/dts/imx6sl-evk.dts
> b/arch/arm/boot/dts/imx6sl-evk.dts index f5e4513..8594d13 100644
> --- a/arch/arm/boot/dts/imx6sl-evk.dts
> +++ b/arch/arm/boot/dts/imx6sl-evk.dts
> @@ -86,55 +86,149 @@
> };
>
> pinctrl_ecspi1: ecspi1grp {
> - fsl,pins = <MX6SL_ECSPI1_PINGRP1>;
> + fsl,pins = <
> + MX6SL_PAD_ECSPI1_MISO__ECSPI1_MISO 0x100b1
> + MX6SL_PAD_ECSPI1_MOSI__ECSPI1_MOSI 0x100b1
> + MX6SL_PAD_ECSPI1_SCLK__ECSPI1_SCLK 0x100b1
> + >;
> };
>
> pinctrl_fec: fecgrp {
> - fsl,pins = <MX6SL_FEC_PINGRP1>;
> + fsl,pins = <
> + MX6SL_PAD_FEC_MDC__FEC_MDC 0x1b0b0
> + MX6SL_PAD_FEC_MDIO__FEC_MDIO 0x1b0b0
> + MX6SL_PAD_FEC_CRS_DV__FEC_RX_DV 0x1b0b0
> + MX6SL_PAD_FEC_RXD0__FEC_RX_DATA0 0x1b0b0
> + MX6SL_PAD_FEC_RXD1__FEC_RX_DATA1 0x1b0b0
> + MX6SL_PAD_FEC_TX_EN__FEC_TX_EN 0x1b0b0
> + MX6SL_PAD_FEC_TXD0__FEC_TX_DATA0 0x1b0b0
> + MX6SL_PAD_FEC_TXD1__FEC_TX_DATA1 0x1b0b0
> + MX6SL_PAD_FEC_REF_CLK__FEC_REF_OUT 0x4001b0a8
> + >;
> };
[... and so on for the other groups ... ]
I'm confused now :-) . Current linux-next [0] shows the pin-settings as part
of imx6sl.dtsi - a way a lot of other architectures organize their pingroups
too, with the board file only referencing the relevant pingroups from the
predefined ones of the soc.
So I guess your move to the pingrp-header moved them out of the imx6sl.dtsi to
the .h and is not part of linux-next; but this patch (and the others in this
series) now moves the definitions into the individual board files. Can't you
just move them back to the soc-dtsi files to prevent each board duplicating
them?
Or I've simply missed previous discussions about this ;-) .
Thanks
Heiko
[0] https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/imx6sl.dtsi#n640
^ permalink raw reply
* [PATCH 1/2] arm64: use num_possible_cpus() instead of NR_CPUS
From: Sudeep Holla @ 2014-01-28 10:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <001d01cf1bc9$452569d0$cf703d70$%han@samsung.com>
On 28/01/14 01:35, Jingoo Han wrote:
> Use num_possible_cpus() instead of direct use of NR_CPUS. Also,
> it fixes the following checkpatch warning.
>
> WARNING: usage of NR_CPUS is often wrong - consider using cpu_possible(), num_possible_cpus(), for_each_possible_cpu(), etc
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
> arch/arm64/kernel/smp.c | 10 +++++-----
> arch/arm64/mm/context.c | 2 +-
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 1b7617a..09ff7d4 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -320,7 +320,7 @@ void __init smp_init_cpus(void)
> * cpu_logical_map was initialized to INVALID_HWID to
> * avoid matching valid MPIDR values.
> */
> - for (i = 1; (i < cpu) && (i < NR_CPUS); i++) {
> + for (i = 1; (i < cpu) && (i < num_possible_cpus()); i++) {
> if (cpu_logical_map(i) == hwid) {
> pr_err("%s: duplicate cpu reg properties in the DT\n",
> dn->full_name);
> @@ -352,7 +352,7 @@ void __init smp_init_cpus(void)
> continue;
> }
>
> - if (cpu >= NR_CPUS)
> + if (cpu >= num_possible_cpus())
Have you tested this patch ? IIUC this will not work as cpu_possible mask is
populated completely and correctly only at the end of this function.
> goto next;
>
> if (cpu_read_ops(dn, cpu) != 0)
> @@ -368,9 +368,9 @@ next:
> }
>
> /* sanity check */
> - if (cpu > NR_CPUS)
> + if (cpu > num_possible_cpus())
> pr_warning("no. of cores (%d) greater than configured maximum of %d - clipping\n",
> - cpu, NR_CPUS);
> + cpu, num_possible_cpus());
>
> if (!bootcpu_valid) {
> pr_err("DT missing boot CPU MPIDR, not enabling secondaries\n");
> @@ -381,7 +381,7 @@ next:
> * All the cpus that made it to the cpu_logical_map have been
> * validated so set them as possible cpus.
> */
> - for (i = 0; i < NR_CPUS; i++)
> + for (i = 0; i < num_possible_cpus(); i++)
> if (cpu_logical_map(i) != INVALID_HWID)
> set_cpu_possible(i, true);
This is what I am referring above, where is possible mask set before this.
If it's already populated correctly then we can remove this completely.
Regards,
Sudeep
^ permalink raw reply
* [PATCH 1/3] mmc: add support for power-on sequencing through DT
From: Ulf Hansson @ 2014-01-28 10:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E700F0.7040708@gmail.com>
On 28 January 2014 01:59, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> On 27.01.2014 11:19, Ulf Hansson wrote:
>>
>> On 26 January 2014 18:26, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>>>
>>> On 21.01.2014 19:34, Tomasz Figa wrote:
>>>>
>>>>
>>>> Hi,
>>>>
>>>> On 20.01.2014 04:56, Olof Johansson wrote:
>>>>>
>>>>>
>>>>> This patch enables support for power-on sequencing of SDIO peripherals
>>>>> through DT.
>>>>>
>>>>> In general, it's quite common that wifi modules and other similar
>>>>> peripherals have several signals in addition to the SDIO interface that
>>>>> needs wiggling before the module will power on. It's common to have a
>>>>> reference clock, one or several power rails and one or several lines
>>>>> for reset/enable type functions.
>>>>>
>>>>> The binding as written today introduces a number of reset gpios,
>>>>> a regulator and a clock specifier. The code will handle up to 2 gpio
>>>>> reset lines, but it's trivial to increase to more than that if needed
>>>>> at some point.
>>>>>
>>>>> Implementation-wise, the MMC core has been changed to handle this
>>>>> during
>>>>> host power up, before the host interface is powered on. I have not yet
>>>>> implemented the power-down side, I wanted people to have a chance for
>>>>> reporting back w.r.t. issues (or comments on the bindings) first.
>>>>>
>>>>> I have not tested the regulator portion, since the system and module
>>>>> I'm working on doesn't need one (Samsung Chromebook with Marvell
>>>>> 8797-based wifi). Testing of those portions (and reporting back) would
>>>>> be appreciated.
>>>>
>>>>
>>>>
>>>> While I fully agree that this is an important problem that needs to be
>>>> solved, I really don't think this is the right way, because:
>>>>
>>>> a) power-up sequence is really specific to the MMC device and often it's
>>>> not simply a matter of switching on one regulator or one clock, e.g.
>>>> specific time constraints need to be met.
>>>>
>>>> b) you can have WLAN chips in which SDIO is just one of the options to
>>>> use as host interface, which may be also HSIC, I2C or UART. Really. See
>>>> [1].
>>>>
>>>> c) this is leaking device specific details to generic host code, which
>>>> isn't really elegant.
>>>>
>>>> Now, to make this a bit more constructive, [2] is a solution that I came
>>>> up with (not perfect either), which simply adds a separate platform
>>>> device for the low level part of the chip. I believe this is a better
>>>> solution because:
>>>>
>>>> a) you can often see such WLAN/BT combo chip as a set of separate
>>>> devices, e.g. SDIO WLAN, UART BT and a simple PMIC or management IC,
>>>> which provides power/reset control, out of band signalling and etc. for
>>>> the first two, so it isn't that bad to have a separate device node for
>>>> the last one,
>>>>
>>>> b) you have full freedom of defining your DT binding with whatever data
>>>> you need, any number of clocks, regulators, GPIOs and even out of band
>>>> interrupts (IMHO the most important one).
>>>>
>>>> c) you can implement power-on, power-off sequences as needed for your
>>>> particular device,
>>>>
>>>> d) you have full separation of device-specific data from MMC core (or
>>>> any other subsystem simply used as a way to perform I/O to the chip).
>>>>
>>>> Now what's missing there is a way to signal the MMC core or any other
>>>> transport that a device showed up and the controller should be woken up
>>>> out of standby and scan of the bus initialized. This could be done by
>>>> explicitly specifying the device as a subnode of the
>>>> MMC/UART/USB(HSIC)/I2C or whatever with a link (phandle) to the power
>>>> controller of the chip or the other way around - a link to the
>>>> MMC/UART/... controller from the power controller node.
>>>
>>>
>>>
>>> I've looked a bit around MMC core code and got some basic idea how things
>>> look. I will definitely need some guidance, or at least some opinions,
>>> from
>>> MMC guys, as some MMC core changes are unavoidable.
>>>
>>> Now, the device-specific code is not really an issue, existing drivers
>>> usually already have their ways of powering the chips on and off, based
>>> on
>>> platform data. Everything needed here is to retrieve needed resources
>>> (GPIOs, clocks, regulators) using DT, which should be trivial.
>>>
>>> The worse part is the interaction between MMC and power controller driver
>>> (the platform driver part of WLAN driver, if you look at brcmfmac as an
>>> example). I believe that we need following things:
>>>
>>> a) A way to tell the MMC controller that there is no card detection
>>> mechanism available on given slot and it also should not be polling the
>>> slot
>>> to check card presence. Something like a "manual card detect" that would
>>> be
>>> triggered by another kernel entity that controls whether the MMC device
>>> is
>>> present (i.e. WLAN driver). We already have "broken-cd" property, but it
>>> only implies the former, wasting time on needless polling.
>>
>>
>> There is already a host capability that I think we could use to handle
>> this. MMC_CAP_NONREMOVABLE, the corresponding DT binding string is
>> "non-removable", and it may be set per host device.
>>
>> Using this cap means the mmc_rescan process that runs to detect new
>> cards, will only be executed once and during boot. So, we need to make
>> sure all resources and powers are provided to the card at this point.
>> Otherwise the card will not be detected.
>
>
> I don't quite like this requirement, especially if you consider
> multi-platform kernels where a lot of drivers is going to be provided as
> modules. WLAN drivers are especially good candidates. This means that even
> if the card is powered off at boot-up, if user (or init system) loads
> appropriate module, which powers the chip on, MMC core must be able to
> notice this.
To be able to detect the card, the WLAN driver doesn't have to be
probed, only the "power controller" driver. I suppose this is were it
becomes a bit tricky.
Somehow the mmc core needs to be involved in the probe process of the
power controller driver. Could perhaps the power controller bus be
located in the mmc core and thus the power controller driver needs to
register itself by using a new API from the mmc core? Similar how SDIO
func driver's register themselves.
I have one concern here though. Unless the SDIO func driver gets
probed, the SDIO card will be kept powered, which is not optimal from
a power management perspective.
To solve this, we need to change the policy about how to handle SDIO
cards after the initialization sequence (mmc_rescan) has been
completed. This will affect SDIO func driver's as well, since at the
moment those expects the card to be fully powered once they are being
probed.
>
>
>> In the SDIO case, to save power, the SDIO func driver may use runtime
>> PM to tell the mmc core power about whether the card needs to be
>> powered. Typically from the WLAN driver's probe() and "interface
>> up/down" the runtime PM reference for the SDIO func device, should be
>> adjusted with pm_runtime_get|put.
>
>
> I need to think a bit more about the power management control flow here. In
> case of such chips I'd tend to look at MMC merely as a host interface, which
> as I said, might be only one of available options. I'm not sure if it should
> be the host interface core that decides whether the whole device should be
> powered off. However there might be a solution that leverages SDIO func
> runtime PM, which wouldn't imply such control flow. Let me reconsider this.
>
Just to clarify things; it is not the "host interface" that decides
whether the whole device should be powered off. This is decided from
the SDIO func driver, by using runtime PM.
The "host interface" still needs to be in control of the power on/off
sequence, since the knowledge about the SDIO spec is required to
handle this.
>
>>
>>>
>>> b) A mechanism to bind the power controller to used MMC slot. Something
>>> like
>>> "mmc-bus = <&mmc2>;" property in device node of the power controller and
>>> a
>>> function like of_find_mmc_controller_by_node(), which would be an MMC
>>> counterpart of I2C's of_find_i2c_adapter_by_node(). To avoid races, it
>>> should probably take a reference on MMC host that would have to be
>>> dropped
>>> explicitly whenever it is not needed anymore.
>>
>>
>> I suppose an "MMC slot" can be translated to "MMC host"?
>
>
> Right.
>
>
>> What I am trying to understand is how the mmc core (or if we push it
>> to be handled from the mmc host's .set_ios callback) shall be able to
>> tell the power controller driver to enable/disable it's resources.
>> Somehow we need the struct device available to handle that. Then I
>> guess operating on it using runtime PM would be a solution that would
>> be quite nice!?
>
>
> As I wrote above, I'm not quite sure about this yet.
>
>
>>>
>>> c) A method to notify the MMC subsystem that card presence has changed.
>>> We
>>> already have something like this in drivers/mmc/core/slot-gpio.c, but
>>> used
>>> for a simple GPIO-based card detection. If the main part of
>>> mmc_gpio_cd_irqt() could be turned into an exported helper, e.g.
>>> mmc_force_card_detect(host) then basically we would have everything
>>> needed.
>>
>>
>> I am not sure I understand why this is needed. I think it would be
>> more convenient to use MMC_CAP_NONREMOVABLE instead as stated earlier.
>> But please elaborate, I might have missed something.
>
>
> See above. I'm not quite convinced that state of MMC interface should
> determine power state of the chip. I can easily imagine a situation where
> the MMC link is powered down (link power management) but the WLAN chip keeps
> operation. Keep in mind that those are usually complete SoCs that can keep
> processing network traffic autonomously and wake-up the application
> processor whenever anything interesting happens using extra out of bounds
> signalling, which might trigger re-enabling the MMC link.
Am not sure I understand what you mean with MMC link here.
We have the VCC regulator that the mmc host driver handles and the
resources by "power controller" driver. Do you want these to be
remained enabled during system suspend or are you saying we might need
even more fine grained power management?
Additionally, as Chris also pointed out in his reply; SDIO func
drivers can prevent the mmc core from powering off the card during
system suspend. Check for the flag, MMC_PM_KEEP_POWER in the code.
Kind regards
Uffe
>
>
>>>
>>> Unfortunately, I don't have more time left for today to create patches
>>> and
>>> test them, so for now, I'd like to hear opinion of MMC maintainers about
>>> this approach. Do you find this acceptable?
>>>
>>> By the way, it seems like slot-gpio.c could replace a lot of custom GPIO
>>> card detection code used in MMC host drivers, e.g. sdhci-s3c. Is there
>>> any
>>> reason why it couldn't?
>>
>>
>> I suppose most host driver's should convert to the slot-gpio API, it's
>> is just a matter of someone to send the patches. :-)
>
>
> OK, great. I'll add conversion of sdhci-s3c to my queue then.
>
> Best regards,
> Tomasz
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] usb: at91-udc: fix irq and iomem resource retrieval
From: boris brezillon @ 2014-01-28 10:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E130FE.1070506@atmel.com>
On 23/01/2014 16:10, Nicolas Ferre wrote:
> On 23/01/2014 15:41, Jean-Jacques Hiblot :
>> When using dt resources retrieval (interrupts and reg properties) there is
>> no predefined order for these resources in the platform dev resource
>> table. Also don't expect the number of resource to be always 2.
>>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
Acked-by: Boris BREZILLON <b.brezillon@overkiz.com>
> Yes, indeed.
>
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>
> Maybe we can also add a "stable" tag to it. Looking at the history of
> this file, I think that we can add a pretty old stable limit... But as
> it only makes sense with DT, I would advice something like this, for the
> 3.4-ish timeframe:
>
> Cc: stable <stable@vger.kernel.org> # 3.4
>
> Bye,
>
>> ---
>> drivers/usb/gadget/at91_udc.c | 10 ----------
>> 1 file changed, 10 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c
>> index 4cc4fd6..dfd2943 100644
>> --- a/drivers/usb/gadget/at91_udc.c
>> +++ b/drivers/usb/gadget/at91_udc.c
>> @@ -1710,16 +1710,6 @@ static int at91udc_probe(struct platform_device *pdev)
>> return -ENODEV;
>> }
>>
>> - if (pdev->num_resources != 2) {
>> - DBG("invalid num_resources\n");
>> - return -ENODEV;
>> - }
>> - if ((pdev->resource[0].flags != IORESOURCE_MEM)
>> - || (pdev->resource[1].flags != IORESOURCE_IRQ)) {
>> - DBG("invalid resource type\n");
>> - return -ENODEV;
>> - }
>> -
>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> if (!res)
>> return -ENXIO;
>>
>
^ permalink raw reply
* [PATCH v2 2/5] clk: sunxi: Add USB clock register defintions
From: Hans de Goede @ 2014-01-28 10:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140128094427.GZ3867@lukather>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi,
On 01/28/2014 10:44 AM, Maxime Ripard wrote:
> On Mon, Jan 27, 2014 at 03:54:14PM +0100, Hans de Goede wrote:
>>>> "allwinner,sun5i-a13-usb-gates-clk" - for usb gates + resets on A13
>>>
>>> Maybe we can just remove the gates from there? Even though they are gates, they are also (a bit) more than that.
>>
>> To be clear you mean s/usb-gates-clk/usb-clk/ right ?
>
> Yep, exactly
>
>>> I guess that means that we will have the OHCI0 gate declared with <&...-gates-clk 6>, while it's actually the first gate for this clock?
>>
>> Correct.
>>
>>> Maybe introducing an offset field in the gates_data would be a good idea, so that we always start from indexing the gates from 0 in the DT?
>>
>> Well for the other "gates" type clks we also have holes in the range, and we always refer to the clk with the bit number in the reg as the clock-cell value.
>
> Yes, we have holes, but I see two majors differences here: - the other gates are just gates, while the usb clocks are a bit more than that.
The usb-clk registers contain more then that, but the bits we are talking
about now are gates.
> - the other gates' gating bits thus all start at bit 0, while here, since it's kind of a "mixed" clock, the gating bits start at bit 6 (on the A20 at least)
Right, still I believe that the consistent thing to do is keeping the
bit-number for the bit in the register controlling the gate as the
specifier. When adding new dts entries / reviewing existing ones
I'm used to matching the specifier to the bit-nr in the data-sheet,
I think making things different just for this one register is counter
productive.
Regards,
Hans
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlLnf80ACgkQF3VEtJrzE/udugCdEDpN65hazG7H+FD45iOVnTY9
548An3dXeF6f8wp5REck5H3gqQPQkIoX
=6yba
-----END PGP SIGNATURE-----
^ permalink raw reply
* [PATCH v2 2/5] clk: sunxi: Add USB clock register defintions
From: Maxime Ripard @ 2014-01-28 9:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E67316.5020906@redhat.com>
On Mon, Jan 27, 2014 at 03:54:14PM +0100, Hans de Goede wrote:
> >> "allwinner,sun5i-a13-usb-gates-clk" - for usb gates + resets on A13
> >
> > Maybe we can just remove the gates from there? Even though they
> > are gates, they are also (a bit) more than that.
>
> To be clear you mean s/usb-gates-clk/usb-clk/ right ?
Yep, exactly
> > I guess that means that we will have the OHCI0 gate declared with
> > <&...-gates-clk 6>, while it's actually the first gate for this
> > clock?
>
> Correct.
>
> > Maybe introducing an offset field in the gates_data would be a
> > good idea, so that we always start from indexing the gates from 0
> > in the DT?
>
> Well for the other "gates" type clks we also have holes in the
> range, and we always refer to the clk with the bit number in the reg
> as the clock-cell value.
Yes, we have holes, but I see two majors differences here:
- the other gates are just gates, while the usb clocks are a bit
more than that.
- the other gates' gating bits thus all start at bit 0, while here,
since it's kind of a "mixed" clock, the gating bits start at bit 6
(on the A20 at least)
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140128/4aeec0e2/attachment.sig>
^ permalink raw reply
* [alsa-devel] [PATCH 4/4] ASoC: tda998x: adjust the audio hw parameters from EDID
From: Takashi Iwai @ 2014-01-28 9:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140127205437.GC11841@sirena.org.uk>
At Mon, 27 Jan 2014 20:54:37 +0000,
Mark Brown wrote:
>
> On Mon, Jan 27, 2014 at 08:49:15PM +0000, Russell King - ARM Linux wrote:
> > On Mon, Jan 27, 2014 at 08:44:41PM +0000, Mark Brown wrote:
>
> > > Can this parsing code be factored out - it (or large parts of it) should
> > > be usable by other HDMI devices shouldn't it?
>
> > Yes, preferably as a generic ALSA helper rather than an ASoC helper -
> > I don't see any need for this to be ASoC specific (I have a pure ALSA
> > driver which has very similar code in it.)
>
> Indeed, definitely ALSA generic - ideally we could factor a lot of the
> integration with the video side out.
Yes, indeed.
OTOH, as discussed recently, we're heading to move from ELD parsing to
more direct communication between video and audio drivers for
HD-audio. ELD will be still provided to user-space, but not evaluated
any longer in the new scenario.
Takashi
^ permalink raw reply
* [alsa-devel] [PATCH RFC v3 0/8] Beaglebone-Black HDMI audio
From: Jean-Francois Moine @ 2014-01-28 9:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E6B42F.4050901@ti.com>
On Mon, 27 Jan 2014 21:31:59 +0200
Jyri Sarha <jsarha@ti.com> wrote:
> I would suggest to leave the CTS_N_K to the current setting (3), unless
> we can change the CTS_N_K on the fly according to the used sample format.
Yes, this is possible:
- the tda998x codec may call the tda998x hdmi in the hw_params()
function, i.e. when the sample format is known, and then,
- the tda998x_audio_update() function may have the audio parameters
(struct snd_pcm_hw_params) as an argument, and CTS_N_K may be set
to either 1, 2 or 3 for SNDRV_PCM_FORMAT_S16_LE / S24_LE and S32_LE.
This is working in my machine. Would it also work for you?
--
Ken ar c'henta? | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
^ permalink raw reply
* [Patch v3 2/2] dmaengine: qcom_bam_dma: Add device tree binding
From: Arnd Bergmann @ 2014-01-28 9:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E772DF.6000604@metafoo.de>
On Tuesday 28 January 2014 10:05:35 Lars-Peter Clausen wrote:
> > +
> > +Clients must use the format described in the dma.txt file, using a three cell
> > +specifier for each channel.
> > +
> > +The three cells in order are:
> > + 1. A phandle pointing to the DMA controller
> > + 2. The channel number
> > + 3. Direction of the fixed unidirectional channel
> > + 0 - Memory to Device
> > + 1 - Device to Memory
> > + 2 - Device to Device
> > +
>
> Why does the direction needs to be specified in specifier? I see two
> options, either the direction per is fixed in hardware. In that case the DMA
> controller node should describe which channel is which direction. Or the
> direction is not fixed in hardware and can be changed at runtime in which
> case it should be set on a per descriptor basis.
Normally the direction is implied by dmaengine_slave_config().
Note that neither the dma slave API nor the generic DT binding
can actually support device-to-device transfers, since this
normally implies using two dma-request lines rather than one.
There might be a case where the direction is required in order
to allocate a channel, because the engine has specialized channels
per direction, and might connect any of them to any dma request
line. This does not seem to be the case for "bam", because
the DMA specifier already contains a specific channel number, not
a request line or slave ID number.
Arnd
^ permalink raw reply
* [Patch v3 2/2] dmaengine: qcom_bam_dma: Add device tree binding
From: Lars-Peter Clausen @ 2014-01-28 9:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390890471-14882-3-git-send-email-agross@codeaurora.org>
On 01/28/2014 07:27 AM, Andy Gross wrote:
> Add device tree binding support for the QCOM BAM DMA driver.
>
> Signed-off-by: Andy Gross <agross@codeaurora.org>
> ---
> .../devicetree/bindings/dma/qcom_bam_dma.txt | 52 ++++++++++++++++++++
> 1 file changed, 52 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
>
> diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> new file mode 100644
> index 0000000..53fd10a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> @@ -0,0 +1,52 @@
> +QCOM BAM DMA controller
> +
> +Required properties:
> +- compatible: Must be "qcom,bam-v1.4.0" for MSM8974 V1
> + Must be "qcom,bam-v1.4.1" for MSM8974 V2
> +- reg: Address range for DMA registers
> +- interrupts: single interrupt for this controller
> +- #dma-cells: must be <2>
> +- clocks: required clock
> +- clock-names: name of clock
> +- qcom,ee : indicates the active Execution Environment identifier (0-7)
> +
> +Example:
> +
> + uart-bam: dma at f9984000 = {
> + compatible = "qcom,bam-v1.4.1";
> + reg = <0xf9984000 0x15000>;
> + interrupts = <0 94 0>;
> + clocks = <&gcc GCC_BAM_DMA_AHB_CLK>;
> + clock-names = "bam_clk";
> + #dma-cells = <2>;
> + qcom,ee = <0>;
> + };
> +
> +Client:
> +Required properties:
> +- dmas: List of dma channel requests
> +- dma-names: Names of aforementioned requested channels
> +
> +Clients must use the format described in the dma.txt file, using a three cell
> +specifier for each channel.
> +
> +The three cells in order are:
> + 1. A phandle pointing to the DMA controller
> + 2. The channel number
> + 3. Direction of the fixed unidirectional channel
> + 0 - Memory to Device
> + 1 - Device to Memory
> + 2 - Device to Device
> +
Why does the direction needs to be specified in specifier? I see two
options, either the direction per is fixed in hardware. In that case the DMA
controller node should describe which channel is which direction. Or the
direction is not fixed in hardware and can be changed at runtime in which
case it should be set on a per descriptor basis.
- Lars
^ permalink raw reply
* [PATCH] clk: si5351: fix non-DT build breakage
From: Sebastian Hesselbarth @ 2014-01-28 8:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390682911-9022-1-git-send-email-sebastian.hesselbarth@gmail.com>
Patch ("clk: si5351: remove variant from platform_data") changed parameters
passed to si5351_dt_parse. While it builds fine with CONFIG_OF, the non-DT
stub of si5351_dt_parse has not been updated and fails to build there.
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Reported-by: Kbuild Test Robot <fengguang.wu@intel.com>
---
Mike,
sorry for that one. Feel free to squash this one into the original patch.
Sebastian
Cc: Mike Turquette <mturquette@linaro.org>
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
---
drivers/clk/clk-si5351.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c
index b95aa09b7aed..e9ee2e12d9cc 100644
--- a/drivers/clk/clk-si5351.c
+++ b/drivers/clk/clk-si5351.c
@@ -1293,7 +1293,8 @@ static int si5351_dt_parse(struct i2c_client *client,
return 0;
}
#else
-static int si5351_dt_parse(struct i2c_client *client)
+static int si5351_dt_parse(struct i2c_client *client,
+ enum si5351_variant variant)
{
return 0;
}
--
1.8.5.2
^ permalink raw reply related
* [PATCH] clk: divider: fix rate calculation for fractional rates
From: Tomi Valkeinen @ 2014-01-28 8:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20131106161911.GA16735@n2100.arm.linux.org.uk>
Hi Mike, Russell,
On 2013-11-06 18:19, Russell King - ARM Linux wrote:
> On Wed, Nov 06, 2013 at 01:48:44PM +0200, Tomi Valkeinen wrote:
>> On 2013-11-06 13:15, Russell King - ARM Linux wrote:
>>> On Wed, Nov 06, 2013 at 01:06:48PM +0200, Tomi Valkeinen wrote:
>>>> This means that the following code works a bit oddly:
>>>>
>>>> rate = clk_round_rate(clk, 123428572);
>>>> clk_set_rate(clk, rate);
>>>
>>> You're right, but the above sequence is quite a crass thing to do. Why?
>>
>> Do you mean that you think the fix is right, but the above example
>> sequence is silly, or that the fix is not needed either?
>
> I think a fix _is) required, because:
>
> rate = clk_get_rate(clk);
> clk_set_rate(clk, rate);
> assert(clk_get_rate(clk) == rate);
>
> If not, there's something quite wrong. However, I am saying that the
> sequence you provided was nevertheless silly - I've seen it in real code
> in the kernel, which is why I've commented about it.
I just ran into this issue again with omap3, and so I'm resurrecting the
thread.
Mike, can you review the patch?
Russell, I'd like to understand why you think the original example is bad:
rate = clk_round_rate(clk, rate);
clk_set_rate(clk, rate);
If the definition of clk_round_rate is basically "clk_set_rate without
actually setting the rate", I agree that the above code is not good as
it might not work correctly.
However, if the following code you gave should work:
rate = clk_get_rate(clk);
clk_set_rate(clk, rate);
assert(clk_get_rate(clk) == rate);
then the original example should also always work, as it's almost the
same as:
/* this is the "round" part */
clk_set_rate(clk, rate);
rate = clk_get_rate(clk);
clk_set_rate(clk, rate);
assert(clk_get_rate(clk) == rate);
Why I'm asking this is that for me (and probably for others also if
you've seen it used in the kernel code) it feels natural to have code like:
rate = clk_round_rate(clk, rate);
/* Verify the rounded rate here to see it's ok for the IP etc */
/* The rate is ok, so set it */
clk_set_rate(clk, rate);
This could be rewritten as:
rounded_rate = clk_round_rate(clk, rate);
/* Verify the rounded rate here to see it's ok for the IP etc */
/* The rounded rate is ok, so set the original rate */
clk_set_rate(clk, rate);
But it just feels unnecessary complication to keep both the original
rate and the rounded rate around.
Tomi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140128/56c20d63/attachment-0001.sig>
^ permalink raw reply
* [PATCHv12] ARM: dts: Add support for the cpuimx35 board from Eukrea and its baseboard.
From: Shawn Guo @ 2014-01-28 8:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1389803532-8516-1-git-send-email-denis@eukrea.com>
On Wed, Jan 15, 2014 at 05:32:12PM +0100, Denis Carikli wrote:
> diff --git a/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi b/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
> new file mode 100644
> index 0000000..303f789
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
> @@ -0,0 +1,59 @@
> +/*
> + * Copyright 2013 Eukr?a Electromatique <denis@eukrea.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + * 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 "imx35.dtsi"
> +
> +/ {
> + model = "Eukrea CPUIMX35";
> + compatible = "eukrea,cpuimx35", "fsl,imx35";
> +
> + memory {
> + reg = <0x80000000 0x8000000>; /* 128M */
> + };
> +};
> +
> +&fec {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_fec>;
> + status = "okay";
> +};
> +
> +&i2c1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_i2c1>;
> + status = "okay";
> +
> + pcf8563 at 51 {
> + compatible = "nxp,pcf8563";
> + reg = <0x51>;
> + };
> +};
> +
> +&iomuxc {
> + imx35-eukrea {
> + pinctrl_fec: fecgrp {
> + fsl,pins = <MX35_FEC_PINGRP1>;
> + };
> +
> + pinctrl_i2c1: i2c1grp {
> + fsl,pins = <MX35_I2C1_PINGRP1>;
> + };
You have these two pingrp in imx35-eukrea-cpuimx35.dtsi ...
> + };
> +};
> +
> +&nfc {
> + nand-bus-width = <8>;
> + nand-ecc-mode = "hw";
> + nand-on-flash-bbt;
> + status = "okay";
> +};
> diff --git a/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard.dts b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard.dts
> new file mode 100644
> index 0000000..23f6fe1
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard.dts
> @@ -0,0 +1,137 @@
> +/*
> + * Copyright 2013 Eukr?a Electromatique <denis@eukrea.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + * 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.
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include "imx35-eukrea-cpuimx35.dtsi"
... imx35-eukrea-cpuimx35.dtsi is being included here ...
> +
> +/ {
> + model = "Eukrea CPUIMX35";
> + compatible = "eukrea,mbimxsd35-baseboard", "eukrea,cpuimx35", "fsl,imx35";
> +
> + gpio_keys {
> + compatible = "gpio-keys";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_bp1>;
> +
> + bp1 {
> + label = "BP1";
> + gpios = <&gpio3 25 GPIO_ACTIVE_LOW>;
> + linux,code = <BTN_MISC>;
> + gpio-key,wakeup;
> + linux,input-type = <1>;
> + };
> + };
> +
> + leds {
> + compatible = "gpio-leds";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_led1>;
> +
> + led1 {
> + label = "led1";
> + gpios = <&gpio3 29 GPIO_ACTIVE_LOW>;
> + linux,default-trigger = "heartbeat";
> + };
> + };
> +};
> +
> +&audmux {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_audmux>;
> + status = "okay";
> +};
> +
> +&esdhc1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_esdhc1>;
> + cd-gpios = <&gpio3 24>;
> + status = "okay";
> +};
> +
> +&i2c1 {
> + tlv320aic23: codec at 1a {
> + compatible = "ti,tlv320aic23";
> + reg = <0x1a>;
> + };
> +};
> +
> +&iomuxc {
> + imx35-eukrea {
> + pinctrl_audmux: audmuxgrp {
> + fsl,pins = <MX35_AUDMUX_PINGRP1>;
> + };
> +
> + pinctrl_bp1: bp1grp {
> + fsl,pins = <MX35_PAD_LD19__GPIO3_25 0x80000000>;
> + };
> +
> + pinctrl_esdhc1: esdhc1grp {
> + fsl,pins = <
> + MX35_ESDHC1_PINGRP1
> + MX35_PAD_LD18__GPIO3_24 0x80000000 /* CD */
> + >;
> + };
> +
> + pinctrl_fec: fecgrp {
> + fsl,pins = <MX35_FEC_PINGRP1>;
> + };
> +
> + pinctrl_i2c1: i2c1grp {
> + fsl,pins = <MX35_I2C1_PINGRP1>;
> + };
... why do you need to have them again?
I spotted this when rebasing this patch on pingrp removal series, and
just dropped them from imx35-eukrea-mbimxsd35-baseboard.dts. Let me
know if I did get it right.
Shawn
> +
> + pinctrl_led1: led1grp {
> + fsl,pins = <MX35_PAD_LD23__GPIO3_29 0x80000000>;
> + };
> +
> + pinctrl_reg_lcd_3v3: reg-lcd-3v3 {
> + fsl,pins = <MX35_PAD_D3_CLS__GPIO1_4 0x80000000>;
> + };
> +
> + pinctrl_uart1: uart1grp {
> + fsl,pins = <
> + MX35_UART1_PINGRP1
> + MX35_UART1_RTSCTS_PINGRP1
> + >;
> + };
> +
> + pinctrl_uart2: uart2grp {
> + fsl,pins = <
> + MX35_UART2_PINGRP1
> + MX35_UART2_RTSCTS_PINGRP1
> + >;
> + };
> + };
> +};
> +
> +&ssi1 {
> + fsl,mode = "i2s-slave";
> + status = "okay";
> +};
> +
> +&uart1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_uart1>;
> + fsl,uart-has-rtscts;
> + status = "okay";
> +};
> +
> +&uart2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_uart2>;
> + fsl,uart-has-rtscts;
> + status = "okay";
> +};
> --
> 1.7.9.5
>
^ 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