* Re: [PATCH] ARM: tegra: dalmore: fix irq trigger type
From: Thierry Reding @ 2014-02-11 20:47 UTC (permalink / raw)
To: Stefan Agner
Cc: swarren, josephl, linux, devicetree, linux-tegra,
linux-arm-kernel, linux-kernel
In-Reply-To: <4fcfcfdbc86c37c6d47ec32cdbd987f3b406e9b5.1392147256.git.stefan@agner.ch>
[-- Attachment #1: Type: text/plain, Size: 1191 bytes --]
On Tue, Feb 11, 2014 at 09:11:32PM +0100, Stefan Agner wrote:
> Trigger type needs to be IRQ_TYPE_LEVEL_HIGH since the interrupt
> signal gets inverted by the PMC (configured by the invert-interrupt
> property).
Isn't the reason the other way around? The PMIC generates a low-level
interrupt, but the GIC can only be configured to accept high-level (or
rising edge) and therefore the nvidia,invert-interrupt property needs to
be set in the PMC node?
One nitpick below.
> diff --git a/arch/arm/boot/dts/tegra114-dalmore.dts b/arch/arm/boot/dts/tegra114-dalmore.dts
[...]
> @@ -888,8 +888,9 @@
> palmas: tps65913@58 {
> compatible = "ti,palmas";
> reg = <0x58>;
> - interrupts = <0 86 IRQ_TYPE_LEVEL_LOW>;
>
> + /* active-low configured by PMC invert-interrupt */
> + interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
I'd prefer to keep the properties grouped as before. interrupts is a
"client" property, whereas #interrupt-cells and interrupt-controller
are "provider" properties.
And I think the comment would be more appropriate in the pmc node, for
the same reason that I think the commit description isn't entirely
accurate.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [Patch v5 2/2] dmaengine: qcom_bam_dma: Add device tree binding
From: Kumar Gala @ 2014-02-11 20:56 UTC (permalink / raw)
To: Andy Gross
Cc: Vinod Koul, Dan Williams, dmaengine, devicetree, linux-kernel,
linux-arm-kernel, linux-arm-msm
In-Reply-To: <1391546556-27702-3-git-send-email-agross@codeaurora.org>
On Feb 4, 2014, at 2:42 PM, Andy Gross <agross@codeaurora.org> 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 | 48 ++++++++++++++++++++
> 1 file changed, 48 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
Acked-by: Kumar Gala <galak@codeaurora.org>
- k
>
> 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..86344f1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> @@ -0,0 +1,48 @@
> +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 <1>
> +- clocks: required clock
> +- clock-names: name of clock
> +- qcom,ee : indicates the active Execution Environment identifier (0-7)
> +
> +Example:
> +
> + uart-bam: dma@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 = <1>;
> + 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 two cell
> +specifier for each channel.
> +
> +The three cells in order are:
> + 1. A phandle pointing to the DMA controller
> + 2. The channel number
> +
> +Example:
> + serial@f991e000 {
> + compatible = "qcom,msm-uart";
> + reg = <0xf991e000 0x1000>
> + <0xf9944000 0x19000>;
> + interrupts = <0 108 0>;
> + clocks = <&gcc GCC_BLSP1_UART2_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>;
> + clock-names = "core", "iface";
> +
> + dmas = <&uart-bam 0>, <&uart-bam 1>;
> + dma-names = "rx", "tx";
> + };
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* Re: [Patch v5 1/2] dmaengine: add Qualcomm BAM dma driver
From: Andy Gross @ 2014-02-11 20:58 UTC (permalink / raw)
To: Vinod Koul
Cc: Dan Williams, dmaengine, devicetree, linux-kernel,
linux-arm-kernel, linux-arm-msm
In-Reply-To: <20140211173048.GP10628@intel.com>
On Tue, Feb 11, 2014 at 11:00:48PM +0530, Vinod Koul wrote:
> On Tue, Feb 04, 2014 at 02:42:35PM -0600, Andy Gross wrote:
> > Add the DMA engine driver for the QCOM Bus Access Manager (BAM) DMA controller
> > found in the MSM 8x74 platforms.
> >
[.....]
> > + * QCOM BAM DMA engine driver
> > + *
> > + * Copyright (c) 2013-2014, The Linux Foundation. All rights reserved.
> This is bit strange, usually copyright is retained by your employer, but I am
> fine with it :)
The short answer is that this is due to legal requirements.
> > +struct bam_desc_hw {
> > + u32 addr; /* Buffer physical address */
> this is where we might have an issue. Is this the DMA register which is 32bit
> wide or you are descibing it so.
> Will this work if you are in 64 bit?
In later implementations, they add 6 more bits to the address field from some of
the reserved bits in the flags. I am not sure what the solution will be for 64
bit, but it may very well be a slightly different descriptor. We'll have to
address this when we add support for chips requiring that functionality.
> > + u16 size; /* Buffer size in bytes */
> > + u16 flags;
> > +};
[....]
> > +
> > + /* reset channel */
> > + writel_relaxed(1, bdev->regs + BAM_P_RST(bchan->id));
> > + writel_relaxed(0, bdev->regs + BAM_P_RST(bchan->id));
> > +
> > + /* don't allow reorder of the channel reset */
> > + wmb();
> Documentation/memory-barriers.txt describes wmb() as a CPU barier but based on
> above you want it to be a compiler barrier then you should do 1st write,
> barrier(), second write.
Sorry, the comment was not clear. I need to reword this. On the Krait,
accesses within a 1k band are not allowed to be reordered. This memory barrier
is to make sure that no reordering is allowed past the call to this function.
> > +
> > + /* make sure hw is initialized when channel is used the first time */
> > + bchan->initialized = 0;
> > +}
> okay why no locks held while reset?
This function is called from 2 places. One is during the freeing of the
channel. The other is when starting the channel (protected via vc.lock). I was
thinking this was safe, but it's easy enough to add a lock around the
bam_reset_channel in the bam_chan_init_hw() and then add a comment to the reset
function saying that a lock is required.
[.....]
> > +
> > + /* set fixed direction and mode, then enable channel */
> > + val = P_EN | P_SYS_MODE;
> > + if (dir == DMA_DEV_TO_MEM)
> > + val |= P_DIRECTION;
> > +
> > + /* make sure the other stores occur before enabling channel */
> > + wmb();
> here again!
I need to reword. The various registers are over 1K boundaries, so the barrier
is required to make sure that all the settings are not allowed to be reordered
past the channel enable.
> > + writel_relaxed(val, bdev->regs + BAM_P_CTRL(bchan->id));
> > +
> > + bchan->initialized = 1;
> > +
> > + /* init FIFO pointers */
> > + bchan->head = 0;
> > + bchan->tail = 0;
> > +}
> > +
> > +/**
> > + * bam_alloc_chan - Allocate channel resources for DMA channel.
> > + * @chan: specified channel
> > + *
> > + * This function allocates the FIFO descriptor memory
> > + */
> > +static int bam_alloc_chan(struct dma_chan *chan)
> > +{
> > + struct bam_chan *bchan = to_bam_chan(chan);
> > + struct bam_device *bdev = bchan->bdev;
> > +
> > + /* allocate FIFO descriptor space, but only if necessary */
> > + if (!bchan->fifo_virt) {
> > + bchan->fifo_virt = dma_alloc_writecombine(bdev->dev,
> > + BAM_DESC_FIFO_SIZE, &bchan->fifo_phys,
> > + GFP_KERNEL);
> > +
> > + if (!bchan->fifo_virt) {
> > + dev_err(bdev->dev, "Failed to allocate desc fifo\n");
> > + return -ENOMEM;
> > + }
> > + }
> > +
> > + return BAM_DESC_FIFO_SIZE;
> But you cna have SW descriptors more than HW ones and issue new ones once HW is
> done with them. Why tie the limit to what HW supports and not create a better
> driver?
Given that the virt-dma only allows me to have one outstanding transaction that
consumes the fifo, and that I allocate descriptors from kzalloc, this would be
as many as you can get until you go OOM.
The slave_sg() doesn't pull from a pool of descriptors. It uses kzalloc. So
what value should I use for return value? Most drivers use 1.
[....]
> > +static void bam_slave_config(struct bam_chan *bchan,
> > + struct dma_slave_config *cfg)
> > +{
> > + struct bam_device *bdev = bchan->bdev;
> > + u32 maxburst;
> > +
> > + if (bchan->slave.direction == DMA_DEV_TO_MEM)
> > + maxburst = bchan->slave.src_maxburst = cfg->src_maxburst;
> > + else
> > + maxburst = bchan->slave.dst_maxburst = cfg->dst_maxburst;
> can you remove usage of slave.direction have save both. I am going to remove
> this member...
>
Yes. no problem.
[....]
> > +
> > +
> > + /* allocate enough room to accomodate the number of entries */
> > + async_desc = kzalloc(sizeof(*async_desc) +
> > + (sg_len * sizeof(struct bam_desc_hw)), GFP_NOWAIT);
> this seems to assume that each sg entry length will not exceed the HW capablity.
> Does engine support any length descriptor, if not you may want to split to
> multiple.
Isn't this what the dma_set_max_seg_size supposed to fix? The client is not
supposed to send segments larger than the max segment you can take. If that is
true, then I have no issues.
> > +
> > + if (!async_desc) {
> > + dev_err(bdev->dev, "failed to allocate async descriptor\n");
> > + goto err_out;
> > + }
> > +
> > + async_desc->num_desc = sg_len;
> > + async_desc->curr_desc = async_desc->desc;
> > + async_desc->dir = direction;
> > +
> > + /* fill in descriptors, align hw descriptor to 8 bytes */
> > + desc = async_desc->desc;
> > + for_each_sg(sgl, sg, sg_len, i) {
> > + if (sg_dma_len(sg) > BAM_MAX_DATA_SIZE) {
> > + dev_err(bdev->dev, "segment exceeds max size\n");
> > + goto err_out;
> gottcha, okay why not split here to multiple descriptors with max size all but
> last?
The client exceeded my dma_max_seg_size. I reject that. If that is not an
allowed solution, then I have to fix my kzalloc size above and split it here.
[....]
> > +
> > + vchan_get_all_descriptors(&bchan->vc, &head);
> > + spin_unlock_irqrestore(&bchan->vc.lock, flag);
> > +
> > + vchan_dma_desc_free_list(&bchan->vc, &head);
> why drop the lock here, i dont think this one needs to be called with lock
> dropped, didnt see it garbbing.
It doesn't need lock. This func just iterates over the head and calls the
desc_free func.
[....]
> > + break;
> > + }
> empty line after each break would be appreciated.
Ok.
[....]
> > + if (bchan->tail + async_desc->xfer_len > MAX_DESCRIPTORS) {
> > + u32 partial = MAX_DESCRIPTORS - bchan->tail;
> > +
> > + memcpy(&fifo[bchan->tail], desc,
> > + partial * sizeof(struct bam_desc_hw));
> > + memcpy(fifo, &desc[partial], (async_desc->xfer_len - partial) *
> > + sizeof(struct bam_desc_hw));
> > + } else {
> > + memcpy(&fifo[bchan->tail], desc,
> > + async_desc->xfer_len * sizeof(struct bam_desc_hw));
> where is the destination memeory located, device or system memory. I think
> later, right?
The destination is the system memory being used for the hw fifo. This was
allocated using dma_alloc_writecombine.
> > + }
> > +
> > + bchan->tail += async_desc->xfer_len;
> > + bchan->tail %= MAX_DESCRIPTORS;
> > +
> > + /* ensure descriptor writes and dma start not reordered */
> > + wmb();
> > + writel_relaxed(bchan->tail * sizeof(struct bam_desc_hw),
> > + bdev->regs + BAM_P_EVNT_REG(bchan->id));
> > +}
> > +
>
> Overall driver loooks fine mostly with few comments as above.
>
> And yes for 2nd patch, would need someone to ACK it before we can appply this
Thanks for the comments. I'll get those done and see about getting the DT
ACK'd.
--
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply
* Re: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
From: Guennadi Liakhovetski @ 2014-02-11 21:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sylwester Nawrocki, Philipp Zabel, Laurent Pinchart,
Russell King - ARM Linux, Rob Herring, Grant Likely, Rob Herring,
Tomi Valkeinen, Kyungmin Park, linux-kernel@vger.kernel.org,
linux-media@vger.kernel.org, devicetree@vger.kernel.org,
Philipp Zabel
In-Reply-To: <20140212024119.7fc96f30.m.chehab@samsung.com>
On Wed, 12 Feb 2014, Mauro Carvalho Chehab wrote:
> Em Tue, 11 Feb 2014 18:22:34 +0100
> Sylwester Nawrocki <s.nawrocki@samsung.com> escreveu:
>
> > (adding Guennadi to Cc)
> >
> > On 11/02/14 17:36, Philipp Zabel wrote:
> > > Am Dienstag, den 11.02.2014, 16:23 +0100 schrieb Laurent Pinchart:
> > >> Hi Russell,
> > >>
> > >> On Tuesday 11 February 2014 14:52:48 Russell King - ARM Linux wrote:
> > >>> On Tue, Feb 11, 2014 at 07:56:33AM -0600, Rob Herring wrote:
> > >>>> On Tue, Feb 11, 2014 at 5:45 AM, Philipp Zabel wrote:
> > >>>>> This allows to reuse the same parser code from outside the V4L2
> > >>>>> framework, most importantly from display drivers. There have been
> > >>>>> patches that duplicate the code (and I am going to send one of my own),
> > >>>>> such as
> > >>>>> http://lists.freedesktop.org/archives/dri-devel/2013-August/043308.html
> > >>>>> and others that parse the same binding in a different way:
> > >>>>> https://www.mail-archive.com/linux-omap@vger.kernel.org/msg100761.html
> > >>>>>
> > >>>>> I think that all common video interface parsing helpers should be moved
> > >>>>> to a single place, outside of the specific subsystems, so that it can
> > >>>>> be reused by all drivers.
> > >>>>
> > >>>> Perhaps that should be done rather than moving to drivers/of now and
> > >>>> then again to somewhere else.
> > >>>
> > >>> Do you have a better suggestion where it should move to?
> > >>>
> > >>> drivers/gpu/drm - no, because v4l2 wants to use it
> > >>> drivers/media/video - no, because DRM drivers want to use it
> > >>> drivers/video - no, because v4l2 and drm drivers want to use it
> > >>
> > >> Just pointing out a missing location (which might be rejected due to similar
> > >> concerns), there's also drivers/media, which isn't V4L-specific.
> > >
> > > Since drivers/Makefile has media/ in obj-y, moving the graph helpers to
> > > drivers/media should technically work.
> > >
> > >>> Maybe drivers/of-graph/ ? Or maybe it's just as good a place to move it
> > >>> into drivers/of ?
> > >
> > > include/media/of_graph.h,
> > > drivers/media/of_graph.c?
> >
> > drivers/media sounds like a good alternative to me.
>
> From my side, I'm ok with putting them at drivers/media. You may add my acked-by
> for such change:
>
> Acked-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cannot see any problems with this
Acked-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Thanks
Guennadi
> > I would just remove also v4l2_of_{parse/get}* and update the users
> > to call of_graph_* directly, there should not be many of them.
> >
> > --
> > Thanks,
> > Sylwester
>
> --
>
> Cheers,
> Mauro
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH] ARM: tegra: dalmore: fix irq trigger type
From: Stefan Agner @ 2014-02-11 21:21 UTC (permalink / raw)
To: Thierry Reding
Cc: swarren-3lzwWm7+Weoh9ZMKESR00Q, josephl-DDmLM1+adcrQT0dZR+AlfA,
linux-lFZ/pmaqli7XmaaqVzeoHQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140211204737.GA1895@mithrandir>
Am 2014-02-11 21:47, schrieb Thierry Reding:
> On Tue, Feb 11, 2014 at 09:11:32PM +0100, Stefan Agner wrote:
>> Trigger type needs to be IRQ_TYPE_LEVEL_HIGH since the interrupt
>> signal gets inverted by the PMC (configured by the invert-interrupt
>> property).
>
> Isn't the reason the other way around? The PMIC generates a low-level
> interrupt, but the GIC can only be configured to accept high-level (or
> rising edge) and therefore the nvidia,invert-interrupt property needs to
> be set in the PMC node?
Hm yes agreed. I should also write the whole story here, maybe this:
The GIC only support high-active interrupts. When using a PMIC with
low-active interrupt, the PMC has to be configured by using the
nvidia,invert-interrupt property in its node.
This fix sets the GIC back to high-active and reverts commit
eca8f98e404934027f84f72882c5e92ffbd9e5f5.
> One nitpick below.
>
>> diff --git a/arch/arm/boot/dts/tegra114-dalmore.dts b/arch/arm/boot/dts/tegra114-dalmore.dts
> [...]
>> @@ -888,8 +888,9 @@
>> palmas: tps65913@58 {
>> compatible = "ti,palmas";
>> reg = <0x58>;
>> - interrupts = <0 86 IRQ_TYPE_LEVEL_LOW>;
>>
>> + /* active-low configured by PMC invert-interrupt */
>> + interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
>
> I'd prefer to keep the properties grouped as before. interrupts is a
> "client" property, whereas #interrupt-cells and interrupt-controller
> are "provider" properties.
>
> And I think the comment would be more appropriate in the pmc node, for
> the same reason that I think the commit description isn't entirely
> accurate.
Well, its kind a question where you are coming from. I read the data
sheet/schemata, and saw that its low-active. Then I went to the PMIC
node and checked how that interrupt is configured, and what you see is
HIGH_ACTIVE. You then think you've found the bug and fix it by setting
it to IRQ_TYPE_LEVEL_LOW. What you don't know is that PMC is in between
and alters the polarity...
^ permalink raw reply
* Re: [PATCH] of: Turn of_match_node into a static inline when CONFIG_OF isn't set
From: Rob Herring @ 2014-02-11 21:30 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Josh Cartwright, Laurent Pinchart,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Grant Likely,
Rob Herring
In-Reply-To: <CAMuHMdVkPLe5986hYPApoQr7oJSb6U2P71=nP+9mtvtaskdHXg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, Feb 11, 2014 at 12:29 PM, Geert Uytterhoeven
<geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> On Tue, Feb 11, 2014 at 7:08 PM, Josh Cartwright <joshc-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>> It sure would be convenient if platform_device had a 'const struct
>> of_device_id *of_id_entry' member similar to the existing struct
>> platform_device_id one, that was set up during platform device matching.
>> Most platform_driver users of of_match_node() would simply go away.
>
> Can't the entry be shared for both platform_device_id and of_device_id?
> Only one of them can be valid at the same time, right?
>
> Ideally, all xxx_device_id look like
>
> struct xxx_device_id {
> ... /* bus-specific ID information */
> kernel_ulong_t driver_data;
> };
>
> This may be formalized in some way, using a base class, but thay may
> require reordering the fields, like:
>
> struct base_device_id {
> kernel_ulong_t driver_data;
> long id[0];
> };
>
I believe this is the reason drivers have to call of_match_device:
commit b1608d69cb804e414d0887140ba08a9398e4e638
Author: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Date: Wed May 18 11:19:24 2011 -0600
drivercore: revert addition of of_match to struct device
Commit b826291c, "drivercore/dt: add a match table pointer to struct
device" added an of_match pointer to struct device to cache the
of_match_table entry discovered at driver match time. This was unsafe
because matching is not an atomic operation with probing a driver. If
two or more drivers are attempted to be matched to a driver at the
same time, then the cached matching entry pointer could get
overwritten.
This patch reverts the of_match cache pointer and reworks all users to
call of_match_device() directly instead.
Signed-off-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2] [media] of: move graph helpers from drivers/media/v4l2-core to drivers/media
From: Philipp Zabel @ 2014-02-11 21:41 UTC (permalink / raw)
To: Russell King - ARM Linux, Mauro Carvalho Chehab
Cc: Grant Likely, Rob Herring, Sylwester Nawrocki, Laurent Pinchart,
Tomi Valkeinen, Kyungmin Park, linux-kernel, linux-media,
devicetree, Guennadi Liakhovetski, Philipp Zabel, Philipp Zabel
From: Philipp Zabel <philipp.zabel@gmail.com>
This patch moves the parsing helpers used to parse connected graphs
in the device tree, like the video interface bindings documented in
Documentation/devicetree/bindings/media/video-interfaces.txt, from
drivers/media/v4l2-core to drivers/media.
This allows to reuse the same parser code from outside the V4L2
framework, most importantly from display drivers.
The functions v4l2_of_get_next_endpoint, v4l2_of_get_remote_port,
and v4l2_of_get_remote_port_parent are moved. They are renamed to
of_graph_get_next_endpoint, of_graph_get_remote_port, and
of_graph_get_remote_port_parent, respectively.
Since there are not that many current users, switch all of them
to the new functions right away.
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
Acked-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
Changes since v1:
- Moved to drivers/media instead of drives/of
- Removed v4l2_of_get_ functions and changed all users in one step
- Keep providing stubs for !OF case
---
drivers/media/Kconfig | 4 +
drivers/media/Makefile | 2 +
drivers/media/i2c/adv7343.c | 4 +-
drivers/media/i2c/mt9p031.c | 4 +-
drivers/media/i2c/s5k5baf.c | 3 +-
drivers/media/i2c/tvp514x.c | 3 +-
drivers/media/i2c/tvp7002.c | 3 +-
drivers/media/of_graph.c | 133 ++++++++++++++++++++++++++
drivers/media/platform/exynos4-is/fimc-is.c | 6 +-
drivers/media/platform/exynos4-is/media-dev.c | 3 +-
drivers/media/platform/exynos4-is/mipi-csis.c | 3 +-
drivers/media/v4l2-core/v4l2-of.c | 117 ----------------------
include/media/of_graph.h | 46 +++++++++
include/media/v4l2-of.h | 24 -----
14 files changed, 202 insertions(+), 153 deletions(-)
create mode 100644 drivers/media/of_graph.c
create mode 100644 include/media/of_graph.h
diff --git a/drivers/media/Kconfig b/drivers/media/Kconfig
index 1d0758a..7681026 100644
--- a/drivers/media/Kconfig
+++ b/drivers/media/Kconfig
@@ -96,6 +96,7 @@ config VIDEO_DEV
tristate
depends on MEDIA_SUPPORT
depends on MEDIA_CAMERA_SUPPORT || MEDIA_ANALOG_TV_SUPPORT || MEDIA_RADIO_SUPPORT
+ select OF_GRAPH if OF
default y
config VIDEO_V4L2_SUBDEV_API
@@ -203,3 +204,6 @@ source "drivers/media/tuners/Kconfig"
source "drivers/media/dvb-frontends/Kconfig"
endif # MEDIA_SUPPORT
+
+config OF_GRAPH
+ bool
diff --git a/drivers/media/Makefile b/drivers/media/Makefile
index 620f275..ff980bd 100644
--- a/drivers/media/Makefile
+++ b/drivers/media/Makefile
@@ -18,6 +18,8 @@ ifeq ($(CONFIG_MEDIA_CONTROLLER),y)
obj-$(CONFIG_MEDIA_SUPPORT) += media.o
endif
+obj-$(CONFIG_OF_GRAPH) += of_graph.o
+
obj-$(CONFIG_VIDEO_DEV) += v4l2-core/
obj-$(CONFIG_DVB_CORE) += dvb-core/
diff --git a/drivers/media/i2c/adv7343.c b/drivers/media/i2c/adv7343.c
index d4e15a6..74a1507 100644
--- a/drivers/media/i2c/adv7343.c
+++ b/drivers/media/i2c/adv7343.c
@@ -28,10 +28,10 @@
#include <linux/of.h>
#include <media/adv7343.h>
+#include <media/of_graph.h>
#include <media/v4l2-async.h>
#include <media/v4l2-device.h>
#include <media/v4l2-ctrls.h>
-#include <media/v4l2-of.h>
#include "adv7343_regs.h"
@@ -410,7 +410,7 @@ adv7343_get_pdata(struct i2c_client *client)
if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
return client->dev.platform_data;
- np = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
+ np = of_graph_get_next_endpoint(client->dev.of_node, NULL);
if (!np)
return NULL;
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index e5ddf47..60f36dc 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -27,9 +27,9 @@
#include <linux/videodev2.h>
#include <media/mt9p031.h>
+#include <media/of_graph.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
-#include <media/v4l2-of.h>
#include <media/v4l2-subdev.h>
#include "aptina-pll.h"
@@ -943,7 +943,7 @@ mt9p031_get_pdata(struct i2c_client *client)
if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
return client->dev.platform_data;
- np = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
+ np = of_graph_get_next_endpoint(client->dev.of_node, NULL);
if (!np)
return NULL;
diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
index 77e10e0..06261ee 100644
--- a/drivers/media/i2c/s5k5baf.c
+++ b/drivers/media/i2c/s5k5baf.c
@@ -25,6 +25,7 @@
#include <linux/slab.h>
#include <media/media-entity.h>
+#include <media/of_graph.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
#include <media/v4l2-subdev.h>
@@ -1855,7 +1856,7 @@ static int s5k5baf_parse_device_node(struct s5k5baf *state, struct device *dev)
if (ret < 0)
return ret;
- node_ep = v4l2_of_get_next_endpoint(node, NULL);
+ node_ep = of_graph_get_next_endpoint(node, NULL);
if (!node_ep) {
dev_err(dev, "no endpoint defined at node %s\n",
node->full_name);
diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
index 83d85df..50062d2 100644
--- a/drivers/media/i2c/tvp514x.c
+++ b/drivers/media/i2c/tvp514x.c
@@ -37,6 +37,7 @@
#include <linux/v4l2-mediabus.h>
#include <linux/of.h>
+#include <media/of_graph.h>
#include <media/v4l2-async.h>
#include <media/v4l2-device.h>
#include <media/v4l2-common.h>
@@ -1068,7 +1069,7 @@ tvp514x_get_pdata(struct i2c_client *client)
if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
return client->dev.platform_data;
- endpoint = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
+ endpoint = of_graph_get_next_endpoint(client->dev.of_node, NULL);
if (!endpoint)
return NULL;
diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
index 912e1cc..7b3201b 100644
--- a/drivers/media/i2c/tvp7002.c
+++ b/drivers/media/i2c/tvp7002.c
@@ -31,6 +31,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/v4l2-dv-timings.h>
+#include <media/of_graph.h>
#include <media/tvp7002.h>
#include <media/v4l2-async.h>
#include <media/v4l2-device.h>
@@ -957,7 +958,7 @@ tvp7002_get_pdata(struct i2c_client *client)
if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
return client->dev.platform_data;
- endpoint = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
+ endpoint = of_graph_get_next_endpoint(client->dev.of_node, NULL);
if (!endpoint)
return NULL;
diff --git a/drivers/media/of_graph.c b/drivers/media/of_graph.c
new file mode 100644
index 0000000..aa526d7
--- /dev/null
+++ b/drivers/media/of_graph.c
@@ -0,0 +1,133 @@
+/*
+ * OF graph binding parsing library
+ *
+ * Copyright (C) 2012 - 2013 Samsung Electronics Co., Ltd.
+ * Author: Sylwester Nawrocki <s.nawrocki@samsung.com>
+ *
+ * Copyright (C) 2012 Renesas Electronics Corp.
+ * Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/types.h>
+
+/**
+ * of_graph_get_next_endpoint() - get next endpoint node
+ * @parent: pointer to the parent device node
+ * @prev: previous endpoint node, or NULL to get first
+ *
+ * Return: An 'endpoint' node pointer with refcount incremented. Refcount
+ * of the passed @prev node is not decremented, the caller have to use
+ * of_node_put() on it when done.
+ */
+struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
+ struct device_node *prev)
+{
+ struct device_node *endpoint;
+ struct device_node *port = NULL;
+
+ if (!parent)
+ return NULL;
+
+ if (!prev) {
+ struct device_node *node;
+ /*
+ * It's the first call, we have to find a port subnode
+ * within this node or within an optional 'ports' node.
+ */
+ node = of_get_child_by_name(parent, "ports");
+ if (node)
+ parent = node;
+
+ port = of_get_child_by_name(parent, "port");
+
+ if (port) {
+ /* Found a port, get an endpoint. */
+ endpoint = of_get_next_child(port, NULL);
+ of_node_put(port);
+ } else {
+ endpoint = NULL;
+ }
+
+ if (!endpoint)
+ pr_err("%s(): no endpoint nodes specified for %s\n",
+ __func__, parent->full_name);
+ of_node_put(node);
+ } else {
+ port = of_get_parent(prev);
+ if (!port)
+ /* Hm, has someone given us the root node ?... */
+ return NULL;
+
+ /* Avoid dropping prev node refcount to 0. */
+ of_node_get(prev);
+ endpoint = of_get_next_child(port, prev);
+ if (endpoint) {
+ of_node_put(port);
+ return endpoint;
+ }
+
+ /* No more endpoints under this port, try the next one. */
+ do {
+ port = of_get_next_child(parent, port);
+ if (!port)
+ return NULL;
+ } while (of_node_cmp(port->name, "port"));
+
+ /* Pick up the first endpoint in this port. */
+ endpoint = of_get_next_child(port, NULL);
+ of_node_put(port);
+ }
+
+ return endpoint;
+}
+EXPORT_SYMBOL(of_graph_get_next_endpoint);
+
+/**
+ * of_graph_get_remote_port_parent() - get remote port's parent node
+ * @node: pointer to a local endpoint device_node
+ *
+ * Return: Remote device node associated with remote endpoint node linked
+ * to @node. Use of_node_put() on it when done.
+ */
+struct device_node *of_graph_get_remote_port_parent(
+ const struct device_node *node)
+{
+ struct device_node *np;
+ unsigned int depth;
+
+ /* Get remote endpoint node. */
+ np = of_parse_phandle(node, "remote-endpoint", 0);
+
+ /* Walk 3 levels up only if there is 'ports' node. */
+ for (depth = 3; depth && np; depth--) {
+ np = of_get_next_parent(np);
+ if (depth == 2 && of_node_cmp(np->name, "ports"))
+ break;
+ }
+ return np;
+}
+EXPORT_SYMBOL(of_graph_get_remote_port_parent);
+
+/**
+ * of_graph_get_remote_port() - get remote port node
+ * @node: pointer to a local endpoint device_node
+ *
+ * Return: Remote port node associated with remote endpoint node linked
+ * to @node. Use of_node_put() on it when done.
+ */
+struct device_node *of_graph_get_remote_port(const struct device_node *node)
+{
+ struct device_node *np;
+
+ /* Get remote endpoint node. */
+ np = of_parse_phandle(node, "remote-endpoint", 0);
+ if (!np)
+ return NULL;
+ return of_get_next_parent(np);
+}
+EXPORT_SYMBOL(of_graph_get_remote_port);
diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c
index 13a4228..6405268 100644
--- a/drivers/media/platform/exynos4-is/fimc-is.c
+++ b/drivers/media/platform/exynos4-is/fimc-is.c
@@ -30,7 +30,7 @@
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/videodev2.h>
-#include <media/v4l2-of.h>
+#include <media/of_graph.h>
#include <media/videobuf2-dma-contig.h>
#include "media-dev.h"
@@ -167,10 +167,10 @@ static int fimc_is_parse_sensor_config(struct fimc_is_sensor *sensor,
u32 tmp = 0;
int ret;
- np = v4l2_of_get_next_endpoint(np, NULL);
+ np = of_graph_get_next_endpoint(np, NULL);
if (!np)
return -ENXIO;
- np = v4l2_of_get_remote_port(np);
+ np = of_graph_get_remote_port(np);
if (!np)
return -ENXIO;
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index c1bce17..11219e2 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -24,6 +24,7 @@
#include <linux/pm_runtime.h>
#include <linux/types.h>
#include <linux/slab.h>
+#include <media/of_graph.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-of.h>
#include <media/media-device.h>
@@ -473,7 +474,7 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
pd->mux_id = (endpoint.port - 1) & 0x1;
- rem = v4l2_of_get_remote_port_parent(ep);
+ rem = of_graph_get_remote_port_parent(ep);
of_node_put(ep);
if (rem == NULL) {
v4l2_info(&fmd->v4l2_dev, "Remote device at %s not found\n",
diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c
index f3c3591..7211523 100644
--- a/drivers/media/platform/exynos4-is/mipi-csis.c
+++ b/drivers/media/platform/exynos4-is/mipi-csis.c
@@ -28,6 +28,7 @@
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/videodev2.h>
+#include <media/of_graph.h>
#include <media/s5p_fimc.h>
#include <media/v4l2-of.h>
#include <media/v4l2-subdev.h>
@@ -762,7 +763,7 @@ static int s5pcsis_parse_dt(struct platform_device *pdev,
&state->max_num_lanes))
return -EINVAL;
- node = v4l2_of_get_next_endpoint(node, NULL);
+ node = of_graph_get_next_endpoint(node, NULL);
if (!node) {
dev_err(&pdev->dev, "No port node at %s\n",
pdev->dev.of_node->full_name);
diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
index 42e3e8a..f919db3 100644
--- a/drivers/media/v4l2-core/v4l2-of.c
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -152,120 +152,3 @@ int v4l2_of_parse_endpoint(const struct device_node *node,
return 0;
}
EXPORT_SYMBOL(v4l2_of_parse_endpoint);
-
-/**
- * v4l2_of_get_next_endpoint() - get next endpoint node
- * @parent: pointer to the parent device node
- * @prev: previous endpoint node, or NULL to get first
- *
- * Return: An 'endpoint' node pointer with refcount incremented. Refcount
- * of the passed @prev node is not decremented, the caller have to use
- * of_node_put() on it when done.
- */
-struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
- struct device_node *prev)
-{
- struct device_node *endpoint;
- struct device_node *port = NULL;
-
- if (!parent)
- return NULL;
-
- if (!prev) {
- struct device_node *node;
- /*
- * It's the first call, we have to find a port subnode
- * within this node or within an optional 'ports' node.
- */
- node = of_get_child_by_name(parent, "ports");
- if (node)
- parent = node;
-
- port = of_get_child_by_name(parent, "port");
-
- if (port) {
- /* Found a port, get an endpoint. */
- endpoint = of_get_next_child(port, NULL);
- of_node_put(port);
- } else {
- endpoint = NULL;
- }
-
- if (!endpoint)
- pr_err("%s(): no endpoint nodes specified for %s\n",
- __func__, parent->full_name);
- of_node_put(node);
- } else {
- port = of_get_parent(prev);
- if (!port)
- /* Hm, has someone given us the root node ?... */
- return NULL;
-
- /* Avoid dropping prev node refcount to 0. */
- of_node_get(prev);
- endpoint = of_get_next_child(port, prev);
- if (endpoint) {
- of_node_put(port);
- return endpoint;
- }
-
- /* No more endpoints under this port, try the next one. */
- do {
- port = of_get_next_child(parent, port);
- if (!port)
- return NULL;
- } while (of_node_cmp(port->name, "port"));
-
- /* Pick up the first endpoint in this port. */
- endpoint = of_get_next_child(port, NULL);
- of_node_put(port);
- }
-
- return endpoint;
-}
-EXPORT_SYMBOL(v4l2_of_get_next_endpoint);
-
-/**
- * v4l2_of_get_remote_port_parent() - get remote port's parent node
- * @node: pointer to a local endpoint device_node
- *
- * Return: Remote device node associated with remote endpoint node linked
- * to @node. Use of_node_put() on it when done.
- */
-struct device_node *v4l2_of_get_remote_port_parent(
- const struct device_node *node)
-{
- struct device_node *np;
- unsigned int depth;
-
- /* Get remote endpoint node. */
- np = of_parse_phandle(node, "remote-endpoint", 0);
-
- /* Walk 3 levels up only if there is 'ports' node. */
- for (depth = 3; depth && np; depth--) {
- np = of_get_next_parent(np);
- if (depth == 2 && of_node_cmp(np->name, "ports"))
- break;
- }
- return np;
-}
-EXPORT_SYMBOL(v4l2_of_get_remote_port_parent);
-
-/**
- * v4l2_of_get_remote_port() - get remote port node
- * @node: pointer to a local endpoint device_node
- *
- * Return: Remote port node associated with remote endpoint node linked
- * to @node. Use of_node_put() on it when done.
- */
-struct device_node *v4l2_of_get_remote_port(const struct device_node *node)
-{
- struct device_node *np;
-
- /* Get remote endpoint node. */
- np = of_parse_phandle(node, "remote-endpoint", 0);
- if (!np)
- return NULL;
- return of_get_next_parent(np);
-}
-EXPORT_SYMBOL(v4l2_of_get_remote_port);
diff --git a/include/media/of_graph.h b/include/media/of_graph.h
new file mode 100644
index 0000000..3bbeb60
--- /dev/null
+++ b/include/media/of_graph.h
@@ -0,0 +1,46 @@
+/*
+ * OF graph binding parsing helpers
+ *
+ * Copyright (C) 2012 - 2013 Samsung Electronics Co., Ltd.
+ * Author: Sylwester Nawrocki <s.nawrocki@samsung.com>
+ *
+ * Copyright (C) 2012 Renesas Electronics Corp.
+ * Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+#ifndef __LINUX_OF_GRAPH_H
+#define __LINUX_OF_GRAPH_H
+
+#ifdef CONFIG_OF
+struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
+ struct device_node *previous);
+struct device_node *of_graph_get_remote_port_parent(
+ const struct device_node *node);
+struct device_node *of_graph_get_remote_port(const struct device_node *node);
+#else
+
+static inline struct device_node *of_graph_get_next_endpoint(
+ const struct device_node *parent,
+ struct device_node *previous)
+{
+ return NULL;
+}
+
+static inline struct device_node *of_graph_get_remote_port_parent(
+ const struct device_node *node)
+{
+ return NULL;
+}
+
+static inline struct device_node *of_graph_get_remote_port(
+ const struct device_node *node)
+{
+ return NULL;
+}
+
+#endif /* CONFIG_OF */
+
+#endif /* __LINUX_OF_GRAPH_H */
diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
index 541cea4..8174282 100644
--- a/include/media/v4l2-of.h
+++ b/include/media/v4l2-of.h
@@ -72,11 +72,6 @@ struct v4l2_of_endpoint {
#ifdef CONFIG_OF
int v4l2_of_parse_endpoint(const struct device_node *node,
struct v4l2_of_endpoint *endpoint);
-struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
- struct device_node *previous);
-struct device_node *v4l2_of_get_remote_port_parent(
- const struct device_node *node);
-struct device_node *v4l2_of_get_remote_port(const struct device_node *node);
#else /* CONFIG_OF */
static inline int v4l2_of_parse_endpoint(const struct device_node *node,
@@ -85,25 +80,6 @@ static inline int v4l2_of_parse_endpoint(const struct device_node *node,
return -ENOSYS;
}
-static inline struct device_node *v4l2_of_get_next_endpoint(
- const struct device_node *parent,
- struct device_node *previous)
-{
- return NULL;
-}
-
-static inline struct device_node *v4l2_of_get_remote_port_parent(
- const struct device_node *node)
-{
- return NULL;
-}
-
-static inline struct device_node *v4l2_of_get_remote_port(
- const struct device_node *node)
-{
- return NULL;
-}
-
#endif /* CONFIG_OF */
#endif /* _V4L2_OF_H */
--
1.9.0.rc3
^ permalink raw reply related
* Re: [RFC PATCH] [media]: of: move graph helpers from drivers/media/v4l2-core to drivers/of
From: Philipp Zabel @ 2014-02-11 21:46 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: Philipp Zabel, Laurent Pinchart, Russell King - ARM Linux,
Rob Herring, Mauro Carvalho Chehab, Grant Likely, Rob Herring,
Tomi Valkeinen, Kyungmin Park, linux-kernel@vger.kernel.org,
linux-media@vger.kernel.org, devicetree@vger.kernel.org,
Philipp Zabel, Guennadi Liakhovetski
In-Reply-To: <52FA5C5A.1090008@samsung.com>
Hi Sylwester,
On Tue, Feb 11, 2014 at 06:22:34PM +0100, Sylwester Nawrocki wrote:
> drivers/media sounds like a good alternative to me.
>
> I would just remove also v4l2_of_{parse/get}* and update the users
> to call of_graph_* directly, there should not be many of them.
For now I'd like to skip v4l2_of_parse_endpoint. The others can just be
copied verbatim, but this one also depends on struct 4l2_of_endpoint,
struct v4l2_of_bus_*, and <media/v4l2-mediabus.h>.
regards
Philipp
^ permalink raw reply
* Re: [PATCH v2] [media] of: move graph helpers from drivers/media/v4l2-core to drivers/media
From: Mauro Carvalho Chehab @ 2014-02-11 21:53 UTC (permalink / raw)
To: Philipp Zabel
Cc: Russell King - ARM Linux, Grant Likely, Rob Herring,
Sylwester Nawrocki, Laurent Pinchart, Tomi Valkeinen,
Kyungmin Park, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Guennadi Liakhovetski,
Philipp Zabel
In-Reply-To: <1392154905-12007-1-git-send-email-p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Em Tue, 11 Feb 2014 22:41:45 +0100
Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> escreveu:
> From: Philipp Zabel <philipp.zabel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> This patch moves the parsing helpers used to parse connected graphs
> in the device tree, like the video interface bindings documented in
> Documentation/devicetree/bindings/media/video-interfaces.txt, from
> drivers/media/v4l2-core to drivers/media.
>
> This allows to reuse the same parser code from outside the V4L2
> framework, most importantly from display drivers.
> The functions v4l2_of_get_next_endpoint, v4l2_of_get_remote_port,
> and v4l2_of_get_remote_port_parent are moved. They are renamed to
> of_graph_get_next_endpoint, of_graph_get_remote_port, and
> of_graph_get_remote_port_parent, respectively.
> Since there are not that many current users, switch all of them
> to the new functions right away.
>
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Acked-by: Mauro Carvalho Chehab <m.chehab-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Acked-by: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>
> ---
> Changes since v1:
> - Moved to drivers/media instead of drives/of
> - Removed v4l2_of_get_ functions and changed all users in one step
> - Keep providing stubs for !OF case
> ---
> drivers/media/Kconfig | 4 +
> drivers/media/Makefile | 2 +
> drivers/media/i2c/adv7343.c | 4 +-
> drivers/media/i2c/mt9p031.c | 4 +-
> drivers/media/i2c/s5k5baf.c | 3 +-
> drivers/media/i2c/tvp514x.c | 3 +-
> drivers/media/i2c/tvp7002.c | 3 +-
> drivers/media/of_graph.c | 133 ++++++++++++++++++++++++++
> drivers/media/platform/exynos4-is/fimc-is.c | 6 +-
> drivers/media/platform/exynos4-is/media-dev.c | 3 +-
> drivers/media/platform/exynos4-is/mipi-csis.c | 3 +-
> drivers/media/v4l2-core/v4l2-of.c | 117 ----------------------
> include/media/of_graph.h | 46 +++++++++
> include/media/v4l2-of.h | 24 -----
> 14 files changed, 202 insertions(+), 153 deletions(-)
> create mode 100644 drivers/media/of_graph.c
> create mode 100644 include/media/of_graph.h
>
> diff --git a/drivers/media/Kconfig b/drivers/media/Kconfig
> index 1d0758a..7681026 100644
> --- a/drivers/media/Kconfig
> +++ b/drivers/media/Kconfig
> @@ -96,6 +96,7 @@ config VIDEO_DEV
> tristate
> depends on MEDIA_SUPPORT
> depends on MEDIA_CAMERA_SUPPORT || MEDIA_ANALOG_TV_SUPPORT || MEDIA_RADIO_SUPPORT
> + select OF_GRAPH if OF
> default y
>
> config VIDEO_V4L2_SUBDEV_API
> @@ -203,3 +204,6 @@ source "drivers/media/tuners/Kconfig"
> source "drivers/media/dvb-frontends/Kconfig"
>
> endif # MEDIA_SUPPORT
> +
> +config OF_GRAPH
> + bool
> diff --git a/drivers/media/Makefile b/drivers/media/Makefile
> index 620f275..ff980bd 100644
> --- a/drivers/media/Makefile
> +++ b/drivers/media/Makefile
> @@ -18,6 +18,8 @@ ifeq ($(CONFIG_MEDIA_CONTROLLER),y)
> obj-$(CONFIG_MEDIA_SUPPORT) += media.o
> endif
>
> +obj-$(CONFIG_OF_GRAPH) += of_graph.o
> +
> obj-$(CONFIG_VIDEO_DEV) += v4l2-core/
> obj-$(CONFIG_DVB_CORE) += dvb-core/
>
> diff --git a/drivers/media/i2c/adv7343.c b/drivers/media/i2c/adv7343.c
> index d4e15a6..74a1507 100644
> --- a/drivers/media/i2c/adv7343.c
> +++ b/drivers/media/i2c/adv7343.c
> @@ -28,10 +28,10 @@
> #include <linux/of.h>
>
> #include <media/adv7343.h>
> +#include <media/of_graph.h>
> #include <media/v4l2-async.h>
> #include <media/v4l2-device.h>
> #include <media/v4l2-ctrls.h>
> -#include <media/v4l2-of.h>
>
> #include "adv7343_regs.h"
>
> @@ -410,7 +410,7 @@ adv7343_get_pdata(struct i2c_client *client)
> if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
> return client->dev.platform_data;
>
> - np = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
> + np = of_graph_get_next_endpoint(client->dev.of_node, NULL);
> if (!np)
> return NULL;
>
> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> index e5ddf47..60f36dc 100644
> --- a/drivers/media/i2c/mt9p031.c
> +++ b/drivers/media/i2c/mt9p031.c
> @@ -27,9 +27,9 @@
> #include <linux/videodev2.h>
>
> #include <media/mt9p031.h>
> +#include <media/of_graph.h>
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-device.h>
> -#include <media/v4l2-of.h>
> #include <media/v4l2-subdev.h>
>
> #include "aptina-pll.h"
> @@ -943,7 +943,7 @@ mt9p031_get_pdata(struct i2c_client *client)
> if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
> return client->dev.platform_data;
>
> - np = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
> + np = of_graph_get_next_endpoint(client->dev.of_node, NULL);
> if (!np)
> return NULL;
>
> diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
> index 77e10e0..06261ee 100644
> --- a/drivers/media/i2c/s5k5baf.c
> +++ b/drivers/media/i2c/s5k5baf.c
> @@ -25,6 +25,7 @@
> #include <linux/slab.h>
>
> #include <media/media-entity.h>
> +#include <media/of_graph.h>
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-device.h>
> #include <media/v4l2-subdev.h>
> @@ -1855,7 +1856,7 @@ static int s5k5baf_parse_device_node(struct s5k5baf *state, struct device *dev)
> if (ret < 0)
> return ret;
>
> - node_ep = v4l2_of_get_next_endpoint(node, NULL);
> + node_ep = of_graph_get_next_endpoint(node, NULL);
> if (!node_ep) {
> dev_err(dev, "no endpoint defined at node %s\n",
> node->full_name);
> diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
> index 83d85df..50062d2 100644
> --- a/drivers/media/i2c/tvp514x.c
> +++ b/drivers/media/i2c/tvp514x.c
> @@ -37,6 +37,7 @@
> #include <linux/v4l2-mediabus.h>
> #include <linux/of.h>
>
> +#include <media/of_graph.h>
> #include <media/v4l2-async.h>
> #include <media/v4l2-device.h>
> #include <media/v4l2-common.h>
> @@ -1068,7 +1069,7 @@ tvp514x_get_pdata(struct i2c_client *client)
> if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
> return client->dev.platform_data;
>
> - endpoint = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
> + endpoint = of_graph_get_next_endpoint(client->dev.of_node, NULL);
> if (!endpoint)
> return NULL;
>
> diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
> index 912e1cc..7b3201b 100644
> --- a/drivers/media/i2c/tvp7002.c
> +++ b/drivers/media/i2c/tvp7002.c
> @@ -31,6 +31,7 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/v4l2-dv-timings.h>
> +#include <media/of_graph.h>
> #include <media/tvp7002.h>
> #include <media/v4l2-async.h>
> #include <media/v4l2-device.h>
> @@ -957,7 +958,7 @@ tvp7002_get_pdata(struct i2c_client *client)
> if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
> return client->dev.platform_data;
>
> - endpoint = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
> + endpoint = of_graph_get_next_endpoint(client->dev.of_node, NULL);
> if (!endpoint)
> return NULL;
>
> diff --git a/drivers/media/of_graph.c b/drivers/media/of_graph.c
> new file mode 100644
> index 0000000..aa526d7
> --- /dev/null
> +++ b/drivers/media/of_graph.c
> @@ -0,0 +1,133 @@
> +/*
> + * OF graph binding parsing library
> + *
> + * Copyright (C) 2012 - 2013 Samsung Electronics Co., Ltd.
> + * Author: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + *
> + * Copyright (C) 2012 Renesas Electronics Corp.
> + * Author: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/types.h>
> +
> +/**
> + * of_graph_get_next_endpoint() - get next endpoint node
> + * @parent: pointer to the parent device node
> + * @prev: previous endpoint node, or NULL to get first
> + *
> + * Return: An 'endpoint' node pointer with refcount incremented. Refcount
> + * of the passed @prev node is not decremented, the caller have to use
> + * of_node_put() on it when done.
> + */
> +struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
> + struct device_node *prev)
> +{
> + struct device_node *endpoint;
> + struct device_node *port = NULL;
> +
> + if (!parent)
> + return NULL;
> +
> + if (!prev) {
> + struct device_node *node;
> + /*
> + * It's the first call, we have to find a port subnode
> + * within this node or within an optional 'ports' node.
> + */
> + node = of_get_child_by_name(parent, "ports");
> + if (node)
> + parent = node;
> +
> + port = of_get_child_by_name(parent, "port");
> +
> + if (port) {
> + /* Found a port, get an endpoint. */
> + endpoint = of_get_next_child(port, NULL);
> + of_node_put(port);
> + } else {
> + endpoint = NULL;
> + }
> +
> + if (!endpoint)
> + pr_err("%s(): no endpoint nodes specified for %s\n",
> + __func__, parent->full_name);
> + of_node_put(node);
> + } else {
> + port = of_get_parent(prev);
> + if (!port)
> + /* Hm, has someone given us the root node ?... */
> + return NULL;
> +
> + /* Avoid dropping prev node refcount to 0. */
> + of_node_get(prev);
> + endpoint = of_get_next_child(port, prev);
> + if (endpoint) {
> + of_node_put(port);
> + return endpoint;
> + }
> +
> + /* No more endpoints under this port, try the next one. */
> + do {
> + port = of_get_next_child(parent, port);
> + if (!port)
> + return NULL;
> + } while (of_node_cmp(port->name, "port"));
> +
> + /* Pick up the first endpoint in this port. */
> + endpoint = of_get_next_child(port, NULL);
> + of_node_put(port);
> + }
> +
> + return endpoint;
> +}
> +EXPORT_SYMBOL(of_graph_get_next_endpoint);
> +
> +/**
> + * of_graph_get_remote_port_parent() - get remote port's parent node
> + * @node: pointer to a local endpoint device_node
> + *
> + * Return: Remote device node associated with remote endpoint node linked
> + * to @node. Use of_node_put() on it when done.
> + */
> +struct device_node *of_graph_get_remote_port_parent(
> + const struct device_node *node)
> +{
> + struct device_node *np;
> + unsigned int depth;
> +
> + /* Get remote endpoint node. */
> + np = of_parse_phandle(node, "remote-endpoint", 0);
> +
> + /* Walk 3 levels up only if there is 'ports' node. */
> + for (depth = 3; depth && np; depth--) {
> + np = of_get_next_parent(np);
> + if (depth == 2 && of_node_cmp(np->name, "ports"))
> + break;
> + }
> + return np;
> +}
> +EXPORT_SYMBOL(of_graph_get_remote_port_parent);
> +
> +/**
> + * of_graph_get_remote_port() - get remote port node
> + * @node: pointer to a local endpoint device_node
> + *
> + * Return: Remote port node associated with remote endpoint node linked
> + * to @node. Use of_node_put() on it when done.
> + */
> +struct device_node *of_graph_get_remote_port(const struct device_node *node)
> +{
> + struct device_node *np;
> +
> + /* Get remote endpoint node. */
> + np = of_parse_phandle(node, "remote-endpoint", 0);
> + if (!np)
> + return NULL;
> + return of_get_next_parent(np);
> +}
> +EXPORT_SYMBOL(of_graph_get_remote_port);
> diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c
> index 13a4228..6405268 100644
> --- a/drivers/media/platform/exynos4-is/fimc-is.c
> +++ b/drivers/media/platform/exynos4-is/fimc-is.c
> @@ -30,7 +30,7 @@
> #include <linux/slab.h>
> #include <linux/types.h>
> #include <linux/videodev2.h>
> -#include <media/v4l2-of.h>
> +#include <media/of_graph.h>
> #include <media/videobuf2-dma-contig.h>
>
> #include "media-dev.h"
> @@ -167,10 +167,10 @@ static int fimc_is_parse_sensor_config(struct fimc_is_sensor *sensor,
> u32 tmp = 0;
> int ret;
>
> - np = v4l2_of_get_next_endpoint(np, NULL);
> + np = of_graph_get_next_endpoint(np, NULL);
> if (!np)
> return -ENXIO;
> - np = v4l2_of_get_remote_port(np);
> + np = of_graph_get_remote_port(np);
> if (!np)
> return -ENXIO;
>
> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
> index c1bce17..11219e2 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/exynos4-is/media-dev.c
> @@ -24,6 +24,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/types.h>
> #include <linux/slab.h>
> +#include <media/of_graph.h>
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-of.h>
> #include <media/media-device.h>
> @@ -473,7 +474,7 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
>
> pd->mux_id = (endpoint.port - 1) & 0x1;
>
> - rem = v4l2_of_get_remote_port_parent(ep);
> + rem = of_graph_get_remote_port_parent(ep);
> of_node_put(ep);
> if (rem == NULL) {
> v4l2_info(&fmd->v4l2_dev, "Remote device at %s not found\n",
> diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c
> index f3c3591..7211523 100644
> --- a/drivers/media/platform/exynos4-is/mipi-csis.c
> +++ b/drivers/media/platform/exynos4-is/mipi-csis.c
> @@ -28,6 +28,7 @@
> #include <linux/slab.h>
> #include <linux/spinlock.h>
> #include <linux/videodev2.h>
> +#include <media/of_graph.h>
> #include <media/s5p_fimc.h>
> #include <media/v4l2-of.h>
> #include <media/v4l2-subdev.h>
> @@ -762,7 +763,7 @@ static int s5pcsis_parse_dt(struct platform_device *pdev,
> &state->max_num_lanes))
> return -EINVAL;
>
> - node = v4l2_of_get_next_endpoint(node, NULL);
> + node = of_graph_get_next_endpoint(node, NULL);
> if (!node) {
> dev_err(&pdev->dev, "No port node at %s\n",
> pdev->dev.of_node->full_name);
> diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
> index 42e3e8a..f919db3 100644
> --- a/drivers/media/v4l2-core/v4l2-of.c
> +++ b/drivers/media/v4l2-core/v4l2-of.c
> @@ -152,120 +152,3 @@ int v4l2_of_parse_endpoint(const struct device_node *node,
> return 0;
> }
> EXPORT_SYMBOL(v4l2_of_parse_endpoint);
> -
> -/**
> - * v4l2_of_get_next_endpoint() - get next endpoint node
> - * @parent: pointer to the parent device node
> - * @prev: previous endpoint node, or NULL to get first
> - *
> - * Return: An 'endpoint' node pointer with refcount incremented. Refcount
> - * of the passed @prev node is not decremented, the caller have to use
> - * of_node_put() on it when done.
> - */
> -struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
> - struct device_node *prev)
> -{
> - struct device_node *endpoint;
> - struct device_node *port = NULL;
> -
> - if (!parent)
> - return NULL;
> -
> - if (!prev) {
> - struct device_node *node;
> - /*
> - * It's the first call, we have to find a port subnode
> - * within this node or within an optional 'ports' node.
> - */
> - node = of_get_child_by_name(parent, "ports");
> - if (node)
> - parent = node;
> -
> - port = of_get_child_by_name(parent, "port");
> -
> - if (port) {
> - /* Found a port, get an endpoint. */
> - endpoint = of_get_next_child(port, NULL);
> - of_node_put(port);
> - } else {
> - endpoint = NULL;
> - }
> -
> - if (!endpoint)
> - pr_err("%s(): no endpoint nodes specified for %s\n",
> - __func__, parent->full_name);
> - of_node_put(node);
> - } else {
> - port = of_get_parent(prev);
> - if (!port)
> - /* Hm, has someone given us the root node ?... */
> - return NULL;
> -
> - /* Avoid dropping prev node refcount to 0. */
> - of_node_get(prev);
> - endpoint = of_get_next_child(port, prev);
> - if (endpoint) {
> - of_node_put(port);
> - return endpoint;
> - }
> -
> - /* No more endpoints under this port, try the next one. */
> - do {
> - port = of_get_next_child(parent, port);
> - if (!port)
> - return NULL;
> - } while (of_node_cmp(port->name, "port"));
> -
> - /* Pick up the first endpoint in this port. */
> - endpoint = of_get_next_child(port, NULL);
> - of_node_put(port);
> - }
> -
> - return endpoint;
> -}
> -EXPORT_SYMBOL(v4l2_of_get_next_endpoint);
> -
> -/**
> - * v4l2_of_get_remote_port_parent() - get remote port's parent node
> - * @node: pointer to a local endpoint device_node
> - *
> - * Return: Remote device node associated with remote endpoint node linked
> - * to @node. Use of_node_put() on it when done.
> - */
> -struct device_node *v4l2_of_get_remote_port_parent(
> - const struct device_node *node)
> -{
> - struct device_node *np;
> - unsigned int depth;
> -
> - /* Get remote endpoint node. */
> - np = of_parse_phandle(node, "remote-endpoint", 0);
> -
> - /* Walk 3 levels up only if there is 'ports' node. */
> - for (depth = 3; depth && np; depth--) {
> - np = of_get_next_parent(np);
> - if (depth == 2 && of_node_cmp(np->name, "ports"))
> - break;
> - }
> - return np;
> -}
> -EXPORT_SYMBOL(v4l2_of_get_remote_port_parent);
> -
> -/**
> - * v4l2_of_get_remote_port() - get remote port node
> - * @node: pointer to a local endpoint device_node
> - *
> - * Return: Remote port node associated with remote endpoint node linked
> - * to @node. Use of_node_put() on it when done.
> - */
> -struct device_node *v4l2_of_get_remote_port(const struct device_node *node)
> -{
> - struct device_node *np;
> -
> - /* Get remote endpoint node. */
> - np = of_parse_phandle(node, "remote-endpoint", 0);
> - if (!np)
> - return NULL;
> - return of_get_next_parent(np);
> -}
> -EXPORT_SYMBOL(v4l2_of_get_remote_port);
> diff --git a/include/media/of_graph.h b/include/media/of_graph.h
> new file mode 100644
> index 0000000..3bbeb60
> --- /dev/null
> +++ b/include/media/of_graph.h
> @@ -0,0 +1,46 @@
> +/*
> + * OF graph binding parsing helpers
> + *
> + * Copyright (C) 2012 - 2013 Samsung Electronics Co., Ltd.
> + * Author: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + *
> + * Copyright (C) 2012 Renesas Electronics Corp.
> + * Author: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + */
> +#ifndef __LINUX_OF_GRAPH_H
> +#define __LINUX_OF_GRAPH_H
> +
> +#ifdef CONFIG_OF
As a matter of consistency, it would be better to test here for
CONFIG_OF_GRAPH instead, to reflect the same symbol that enables such
functions as used on Kconfig/Makefile.
> +struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
> + struct device_node *previous);
> +struct device_node *of_graph_get_remote_port_parent(
> + const struct device_node *node);
> +struct device_node *of_graph_get_remote_port(const struct device_node *node);
> +#else
> +
> +static inline struct device_node *of_graph_get_next_endpoint(
> + const struct device_node *parent,
> + struct device_node *previous)
> +{
> + return NULL;
> +}
> +
> +static inline struct device_node *of_graph_get_remote_port_parent(
> + const struct device_node *node)
> +{
> + return NULL;
> +}
> +
> +static inline struct device_node *of_graph_get_remote_port(
> + const struct device_node *node)
> +{
> + return NULL;
> +}
> +
> +#endif /* CONFIG_OF */
> +
> +#endif /* __LINUX_OF_GRAPH_H */
> diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
> index 541cea4..8174282 100644
> --- a/include/media/v4l2-of.h
> +++ b/include/media/v4l2-of.h
> @@ -72,11 +72,6 @@ struct v4l2_of_endpoint {
> #ifdef CONFIG_OF
> int v4l2_of_parse_endpoint(const struct device_node *node,
> struct v4l2_of_endpoint *endpoint);
> -struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
> - struct device_node *previous);
> -struct device_node *v4l2_of_get_remote_port_parent(
> - const struct device_node *node);
> -struct device_node *v4l2_of_get_remote_port(const struct device_node *node);
> #else /* CONFIG_OF */
>
> static inline int v4l2_of_parse_endpoint(const struct device_node *node,
> @@ -85,25 +80,6 @@ static inline int v4l2_of_parse_endpoint(const struct device_node *node,
> return -ENOSYS;
> }
>
> -static inline struct device_node *v4l2_of_get_next_endpoint(
> - const struct device_node *parent,
> - struct device_node *previous)
> -{
> - return NULL;
> -}
> -
> -static inline struct device_node *v4l2_of_get_remote_port_parent(
> - const struct device_node *node)
> -{
> - return NULL;
> -}
> -
> -static inline struct device_node *v4l2_of_get_remote_port(
> - const struct device_node *node)
> -{
> - return NULL;
> -}
> -
> #endif /* CONFIG_OF */
>
> #endif /* _V4L2_OF_H */
--
Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] of: Turn of_match_node into a static inline when CONFIG_OF isn't set
From: Josh Cartwright @ 2014-02-11 21:55 UTC (permalink / raw)
To: Rob Herring
Cc: Geert Uytterhoeven, Laurent Pinchart,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Grant Likely,
Rob Herring
In-Reply-To: <CAL_JsqKnsr-9nSjCGvroC0nDK36vQUnwMWj=cPjvUbRymmHFOQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, Feb 11, 2014 at 03:30:33PM -0600, Rob Herring wrote:
> On Tue, Feb 11, 2014 at 12:29 PM, Geert Uytterhoeven
> <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> > On Tue, Feb 11, 2014 at 7:08 PM, Josh Cartwright <joshc-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> >> It sure would be convenient if platform_device had a 'const struct
> >> of_device_id *of_id_entry' member similar to the existing struct
> >> platform_device_id one, that was set up during platform device matching.
> >> Most platform_driver users of of_match_node() would simply go away.
> >
> > Can't the entry be shared for both platform_device_id and of_device_id?
> > Only one of them can be valid at the same time, right?
> >
[..]
>
> I believe this is the reason drivers have to call of_match_device:
>
> commit b1608d69cb804e414d0887140ba08a9398e4e638
> Author: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> Date: Wed May 18 11:19:24 2011 -0600
>
> drivercore: revert addition of of_match to struct device
>
> Commit b826291c, "drivercore/dt: add a match table pointer to struct
> device" added an of_match pointer to struct device to cache the
> of_match_table entry discovered at driver match time. This was unsafe
> because matching is not an atomic operation with probing a driver. If
> two or more drivers are attempted to be matched to a driver at the
> same time, then the cached matching entry pointer could get
> overwritten.
>
> This patch reverts the of_match cache pointer and reworks all users to
> call of_match_device() directly instead.
>
> Signed-off-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Interesting, thanks for the history! I'm wondering if this same problem
exists for the existing platform_device_id cached pointer as well.
Okay, so maybe caching a pointer in the device isn't the best option,
what if we considered extending the platform_driver callbacks to include
a set of per-method (?) probe callbacks which do provide a handle to
matched identifiers.
In the case of a totally contrived platform_driver supporting ACPI, OF,
and !OF configurations, it might look something like:
static const struct of_device_id acme_of_table[] = {
/* ... */
{ },
};
MODULE_DEVICE_TABLE(of, acme_of_table);
static int acme_probe_of(struct platform_device *pdev,
const struct of_device_id *id)
{
/* ... */
return 0;
}
static const struct acpi_device_id acme_acpi_table[] = {
/* ... */
{ },
};
MODULE_DEVICE_TABLE(acpi, acme_acpi_table);
static int acme_probe_acpi(struct platform_device *pdev,
const struct acpi_device_id *id)
{
/* ... */
return 0;
}
static const struct platform_device_id acme_platform_table[] = {
/* ... */
{ },
};
MODULE_DEVICE_TABLE(platform, acme_platform_table);
static int acme_probe_acpi(struct platform_device *pdev,
const struct platform_device_id *id)
{
/* ... */
return 0;
}
static int acme_probe_name(struct platform_device *pdev)
{
/* ... */
return 0;
}
static struct platform_driver acme_driver = {
.probe_of = acme_probe_of,
.probe_acpi = acme_probe_acpi,
.probe_platform = acme_probe_platform,
.probe_name = acme_probe_name,
.remove = acme_remove,
.driver = {
.name = "acme",
.of_match_table = of_match_ptr(acme_of_table),
.acpi_match_table = ACPI_PTR(acme_acpi_table),
},
};
module_platform_driver(acme_driver);
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
From: Kumar Gala @ 2014-02-11 22:33 UTC (permalink / raw)
To: Stephen N Chivers
Cc: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, Chris Proctor, devicetree,
sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <OF6F6A0029.3B20EF5B-ONCA257C7C.0071BFDA-CA257C7C.00732AD3-SmukeSwxQOQ@public.gmane.org>
On Feb 11, 2014, at 2:57 PM, Stephen N Chivers <schivers-znpAAEhiOVUQrrorzV6ljw@public.gmane.org> wrote:
> I have been trial booting a 3.14-rc2 kernel for a 85xx platform
> (dtbImage).
>
> After mounting the root filesystem there are no messages from the init
> scripts
> and the serial console is not available for login.
>
> In the kernel log messages there is:
>
> of_serial f1004500.serial: Unknown serial port found, ignored.
>
> The serial nodes in boards dts file are specified as:
>
> serial0: serial@4500 {
> cell-index = <0>;
> device_type = "serial";
> compatible = "fsl,ns16550", "ns16550";
> reg = <0x4500 0x100>;
> clock-frequency = <0>;
> interrupts = <0x2a 0x2>;
> interrupt-parent = <&mpic>;
> };
>
> Reversing the order of the compatible:
>
> compatible = "ns16550", "fsl,ns16550";
>
> restores the serial console.
>
> Linux-3.13 does not have this behaviour.
>
> There are 49 dts files in Linux-3.14-rc2 that have the fsl,ns16550
> compatible first.
Hmm,
Wondering if this caused the issue:
commit 105353145eafb3ea919f5cdeb652a9d8f270228e
Author: Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Tue Dec 3 14:52:00 2013 +0100
OF: base: match each node compatible against all given matches first
- k
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
From: Sebastian Hesselbarth @ 2014-02-11 22:51 UTC (permalink / raw)
To: Kumar Gala, Stephen N Chivers
Cc: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, Chris Proctor, devicetree,
Arnd Bergmann
In-Reply-To: <63AEBD99-AA87-4FD7-BBDA-0CE419959F14-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
On 02/11/2014 11:33 PM, Kumar Gala wrote:
>
> On Feb 11, 2014, at 2:57 PM, Stephen N Chivers <schivers-znpAAEhiOVUQrrorzV6ljw@public.gmane.org> wrote:
>
>> I have been trial booting a 3.14-rc2 kernel for a 85xx platform
>> (dtbImage).
>>
>> After mounting the root filesystem there are no messages from the init
>> scripts
>> and the serial console is not available for login.
>>
>> In the kernel log messages there is:
>>
>> of_serial f1004500.serial: Unknown serial port found, ignored.
>>
>> The serial nodes in boards dts file are specified as:
>>
>> serial0: serial@4500 {
>> cell-index = <0>;
>> device_type = "serial";
>> compatible = "fsl,ns16550", "ns16550";
>> reg = <0x4500 0x100>;
>> clock-frequency = <0>;
>> interrupts = <0x2a 0x2>;
>> interrupt-parent = <&mpic>;
>> };
>>
>> Reversing the order of the compatible:
>>
>> compatible = "ns16550", "fsl,ns16550";
>>
>> restores the serial console.
>>
>> Linux-3.13 does not have this behaviour.
>>
>> There are 49 dts files in Linux-3.14-rc2 that have the fsl,ns16550
>> compatible first.
>
> Hmm,
>
> Wondering if this caused the issue:
>
> commit 105353145eafb3ea919f5cdeb652a9d8f270228e
> Author: Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Date: Tue Dec 3 14:52:00 2013 +0100
>
> OF: base: match each node compatible against all given matches first
[adding Arnd on Cc]
Could be. I checked tty/serial/of_serial.c and it does not provide a
compatible for "fsl,ns16550". Does reverting the patch fix the issue
observed?
I don't think the missing compatible is causing it, but of_serial
provides a DT match for .type = "serial" just to fail later on
with the error seen above.
The commit in question reorders of_match_device in a way that match
table order is not relevant anymore. This can cause it to match
.type = "serial" first here.
Rather than touching the commit, I suggest to remove the problematic
.type = "serial" from the match table. It is of no use anyway.
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] of: Turn of_match_node into a static inline when CONFIG_OF isn't set
From: Rob Herring @ 2014-02-11 23:14 UTC (permalink / raw)
To: Josh Cartwright
Cc: Geert Uytterhoeven, Laurent Pinchart,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Grant Likely,
Rob Herring
In-Reply-To: <20140211215519.GJ841-OP5zVEFNDbfdOxZ39nK119BPR1lH4CV8@public.gmane.org>
On Tue, Feb 11, 2014 at 3:55 PM, Josh Cartwright <joshc-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> On Tue, Feb 11, 2014 at 03:30:33PM -0600, Rob Herring wrote:
>> On Tue, Feb 11, 2014 at 12:29 PM, Geert Uytterhoeven
>> <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
>> > On Tue, Feb 11, 2014 at 7:08 PM, Josh Cartwright <joshc-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>> >> It sure would be convenient if platform_device had a 'const struct
>> >> of_device_id *of_id_entry' member similar to the existing struct
>> >> platform_device_id one, that was set up during platform device matching.
>> >> Most platform_driver users of of_match_node() would simply go away.
>> >
>> > Can't the entry be shared for both platform_device_id and of_device_id?
>> > Only one of them can be valid at the same time, right?
>> >
> [..]
>>
>> I believe this is the reason drivers have to call of_match_device:
>>
>> commit b1608d69cb804e414d0887140ba08a9398e4e638
>> Author: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>> Date: Wed May 18 11:19:24 2011 -0600
>>
>> drivercore: revert addition of of_match to struct device
>>
>> Commit b826291c, "drivercore/dt: add a match table pointer to struct
>> device" added an of_match pointer to struct device to cache the
>> of_match_table entry discovered at driver match time. This was unsafe
>> because matching is not an atomic operation with probing a driver. If
>> two or more drivers are attempted to be matched to a driver at the
>> same time, then the cached matching entry pointer could get
>> overwritten.
>>
>> This patch reverts the of_match cache pointer and reworks all users to
>> call of_match_device() directly instead.
>>
>> Signed-off-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>
> Interesting, thanks for the history! I'm wondering if this same problem
> exists for the existing platform_device_id cached pointer as well.
>
> Okay, so maybe caching a pointer in the device isn't the best option,
> what if we considered extending the platform_driver callbacks to include
> a set of per-method (?) probe callbacks which do provide a handle to
> matched identifiers.
>
> In the case of a totally contrived platform_driver supporting ACPI, OF,
> and !OF configurations, it might look something like:
>
> static const struct of_device_id acme_of_table[] = {
> /* ... */
> { },
> };
> MODULE_DEVICE_TABLE(of, acme_of_table);
>
> static int acme_probe_of(struct platform_device *pdev,
> const struct of_device_id *id)
I don't think this is the right direction. You might want to look at
of_platform_driver in git history...
We still have something like this for macio_bus though.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
From: Stephen N Chivers @ 2014-02-11 23:38 UTC (permalink / raw)
To: Sebastian Hesselbarth
Cc: Chris Proctor, Arnd Bergmann, devicetree, Stephen N Chivers,
linuxppc-dev
In-Reply-To: <52FAA97F.4060600@gmail.com>
Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> wrote on
02/12/2014 09:51:43 AM:
> From: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> To: Kumar Gala <galak@kernel.crashing.org>, Stephen N Chivers
> <schivers@csc.com.au>
> Cc: linuxppc-dev@lists.ozlabs.org, Chris Proctor
> <cproctor@csc.com.au>, devicetree <devicetree@vger.kernel.org>, Arnd
> Bergmann <arnd@arndb.de>
> Date: 02/12/2014 09:51 AM
> Subject: Re: Linux-3.14-rc2: Order of serial node compatibles in DTS
files.
>
> On 02/11/2014 11:33 PM, Kumar Gala wrote:
> >
> > On Feb 11, 2014, at 2:57 PM, Stephen N Chivers <schivers@csc.com.au>
wrote:
> >
> >> I have been trial booting a 3.14-rc2 kernel for a 85xx platform
> >> (dtbImage).
> >>
> >> After mounting the root filesystem there are no messages from the
init
> >> scripts
> >> and the serial console is not available for login.
> >>
> >> In the kernel log messages there is:
> >>
> >> of_serial f1004500.serial: Unknown serial port found, ignored.
> >>
> >> The serial nodes in boards dts file are specified as:
> >>
> >> serial0: serial@4500 {
> >> cell-index = <0>;
> >> device_type = "serial";
> >> compatible = "fsl,ns16550", "ns16550";
> >> reg = <0x4500 0x100>;
> >> clock-frequency = <0>;
> >> interrupts = <0x2a 0x2>;
> >> interrupt-parent = <&mpic>;
> >> };
> >>
> >> Reversing the order of the compatible:
> >>
> >> compatible = "ns16550", "fsl,ns16550";
> >>
> >> restores the serial console.
> >>
> >> Linux-3.13 does not have this behaviour.
> >>
> >> There are 49 dts files in Linux-3.14-rc2 that have the fsl,ns16550
> >> compatible first.
> >
> > Hmm,
> >
> > Wondering if this caused the issue:
> >
> > commit 105353145eafb3ea919f5cdeb652a9d8f270228e
> > Author: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> > Date: Tue Dec 3 14:52:00 2013 +0100
> >
> > OF: base: match each node compatible against all given matches
first
>
> [adding Arnd on Cc]
>
> Could be. I checked tty/serial/of_serial.c and it does not provide a
> compatible for "fsl,ns16550". Does reverting the patch fix the issue
> observed?
>
> I don't think the missing compatible is causing it, but of_serial
> provides a DT match for .type = "serial" just to fail later on
> with the error seen above.
>
> The commit in question reorders of_match_device in a way that match
> table order is not relevant anymore. This can cause it to match
> .type = "serial" first here.
>
> Rather than touching the commit, I suggest to remove the problematic
> .type = "serial" from the match table. It is of no use anyway.
Deleting the "serial" line from the match table fixes the problem.
I tested it for both orderings of compatible.
>
> Sebastian
Thanks,
Stephen Chivers,
CSC Australia Pty. Ltd.
^ permalink raw reply
* Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
From: Scott Wood @ 2014-02-11 23:41 UTC (permalink / raw)
To: Sebastian Hesselbarth
Cc: Kumar Gala, Stephen N Chivers, Chris Proctor,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, Arnd Bergmann, devicetree
In-Reply-To: <52FAA97F.4060600-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Tue, 2014-02-11 at 23:51 +0100, Sebastian Hesselbarth wrote:
> On 02/11/2014 11:33 PM, Kumar Gala wrote:
> > Hmm,
> >
> > Wondering if this caused the issue:
> >
> > commit 105353145eafb3ea919f5cdeb652a9d8f270228e
> > Author: Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Date: Tue Dec 3 14:52:00 2013 +0100
> >
> > OF: base: match each node compatible against all given matches first
>
> [adding Arnd on Cc]
>
> Could be. I checked tty/serial/of_serial.c and it does not provide a
> compatible for "fsl,ns16550". Does reverting the patch fix the issue
> observed?
>
> I don't think the missing compatible is causing it, but of_serial
> provides a DT match for .type = "serial" just to fail later on
> with the error seen above.
>
> The commit in question reorders of_match_device in a way that match
> table order is not relevant anymore. This can cause it to match
> .type = "serial" first here.
>
> Rather than touching the commit, I suggest to remove the problematic
> .type = "serial" from the match table. It is of no use anyway.
Regardless of whether .type = "serial" gets removed, it seems wrong for
of_match_node() to accept a .type-only match (or .name, or anything else
that doesn't involve .compatible) before it accepts a compatible match
other than the first in the compatible property.
-Scott
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
From: Sebastian Hesselbarth @ 2014-02-11 23:43 UTC (permalink / raw)
To: Stephen N Chivers
Cc: Arnd Bergmann, Chris Proctor, devicetree, Kumar Gala,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <OFB203CA90.B048F8AA-ONCA257C7C.0081A816-CA257C7C.0081DCE3-SmukeSwxQOQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2132 bytes --]
On 02/12/2014 12:38 AM, Stephen N Chivers wrote:
> Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote on
>> On 02/11/2014 11:33 PM, Kumar Gala wrote:
>>> On Feb 11, 2014, at 2:57 PM, Stephen N Chivers <schivers-znpAAEhiOVUQrrorzV6ljw@public.gmane.org> wrote:
>>>> I have been trial booting a 3.14-rc2 kernel for a 85xx platform
>>>> (dtbImage).
[...]
>>>>
>>>> of_serial f1004500.serial: Unknown serial port found, ignored.
>>>>
>>>> The serial nodes in boards dts file are specified as:
>>>>
>>>> serial0: serial@4500 {
>>>> cell-index = <0>;
>>>> device_type = "serial";
>>>> compatible = "fsl,ns16550", "ns16550";
>>>> reg = <0x4500 0x100>;
>>>> clock-frequency = <0>;
>>>> interrupts = <0x2a 0x2>;
>>>> interrupt-parent = <&mpic>;
>>>> };
>>>
>>> Wondering if this caused the issue:
>>>
>>> commit 105353145eafb3ea919f5cdeb652a9d8f270228e
>>> Author: Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> Date: Tue Dec 3 14:52:00 2013 +0100
>>>
>>> OF: base: match each node compatible against all given matches first
>>
[...]
>>
>> I don't think the missing compatible is causing it, but of_serial
>> provides a DT match for .type = "serial" just to fail later on
>> with the error seen above.
>>
>> The commit in question reorders of_match_device in a way that match
>> table order is not relevant anymore. This can cause it to match
>> .type = "serial" first here.
>>
>> Rather than touching the commit, I suggest to remove the problematic
>> .type = "serial" from the match table. It is of no use anyway.
> Deleting the "serial" line from the match table fixes the problem.
> I tested it for both orderings of compatible.
I revert my statement about removing anything from of_serial.c. Instead
we should try to prefer matches with compatibles over type/name without
compatibles. Something like the patch below (compile tested only)
[-- Attachment #2: of_base_match.patch --]
[-- Type: text/x-patch, Size: 1484 bytes --]
diff --git a/drivers/of/base.c b/drivers/of/base.c
index ff85450d5683..60da53b385ff 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -734,6 +734,7 @@ static
const struct of_device_id *__of_match_node(const struct of_device_id *matches,
const struct device_node *node)
{
+ const struct of_device_id *m;
const char *cp;
int cplen, l;
@@ -742,15 +743,15 @@ const struct of_device_id *__of_match_node(const struct of_device_id *matches,
cp = __of_get_property(node, "compatible", &cplen);
do {
- const struct of_device_id *m = matches;
+ m = matches;
/* Check against matches with current compatible string */
while (m->name[0] || m->type[0] || m->compatible[0]) {
int match = 1;
- if (m->name[0])
+ if (m->name[0] && m->compatible[0])
match &= node->name
&& !strcmp(m->name, node->name);
- if (m->type[0])
+ if (m->type[0] && m->compatible[0])
match &= node->type
&& !strcmp(m->type, node->type);
if (m->compatible[0])
@@ -770,6 +771,21 @@ const struct of_device_id *__of_match_node(const struct of_device_id *matches,
}
} while (cp && (cplen > 0));
+ /* Check against matches without compatible string */
+ m = matches;
+ while (m->name[0] || m->type[0]) {
+ int match = 1;
+ if (m->name[0])
+ match &= node->name
+ && !strcmp(m->name, node->name);
+ if (m->type[0])
+ match &= node->type
+ && !strcmp(m->type, node->type);
+ if (match)
+ return m;
+ m++;
+ }
+
return NULL;
}
^ permalink raw reply related
* Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
From: Sebastian Hesselbarth @ 2014-02-11 23:46 UTC (permalink / raw)
To: Scott Wood
Cc: Kumar Gala, Stephen N Chivers, Chris Proctor,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, Arnd Bergmann, devicetree
In-Reply-To: <1392162080.6733.404.camel-88ow+0ZRuxG2UiBs7uKeOtHuzzzSOjJt@public.gmane.org>
On 02/12/2014 12:41 AM, Scott Wood wrote:
> On Tue, 2014-02-11 at 23:51 +0100, Sebastian Hesselbarth wrote:
>> On 02/11/2014 11:33 PM, Kumar Gala wrote:
>>> Hmm,
>>>
>>> Wondering if this caused the issue:
>>>
>>> commit 105353145eafb3ea919f5cdeb652a9d8f270228e
>>> Author: Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> Date: Tue Dec 3 14:52:00 2013 +0100
>>>
>>> OF: base: match each node compatible against all given matches first
>>
>> [adding Arnd on Cc]
>>
>> Could be. I checked tty/serial/of_serial.c and it does not provide a
>> compatible for "fsl,ns16550". Does reverting the patch fix the issue
>> observed?
>>
>> I don't think the missing compatible is causing it, but of_serial
>> provides a DT match for .type = "serial" just to fail later on
>> with the error seen above.
>>
>> The commit in question reorders of_match_device in a way that match
>> table order is not relevant anymore. This can cause it to match
>> .type = "serial" first here.
>>
>> Rather than touching the commit, I suggest to remove the problematic
>> .type = "serial" from the match table. It is of no use anyway.
>
> Regardless of whether .type = "serial" gets removed, it seems wrong for
> of_match_node() to accept a .type-only match (or .name, or anything else
> that doesn't involve .compatible) before it accepts a compatible match
> other than the first in the compatible property.
Right, I thought about it and came to the same conclusion. I sent a
patch a second ago to prefer .compatible != NULL matches over those
with .compatible == NULL.
Would be great if Stephen can re-test that. If it solves the issue, I
can send a patch tomorrow.
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 0/3] of: functions to count number of elements and convert regulators
From: Heiko Stübner @ 2014-02-11 23:59 UTC (permalink / raw)
To: grant.likely; +Cc: robh+dt, devicetree, linux-kernel, Liam Girdwood, Mark Brown
In a different thread [0] Mark Rutland suggested that drivers should not
repeatedly open-code the counting of array elements in a property as well
as handling the format and endianes of the DTB, as these should be limited
to the of_ helper functions.
Therefore the first patch introduces a set of helper functions for counting
the number of u8,...,u64 elements in a property.
The second and third patch convert the two regulator drivers that use this
pattern to instead use both of_property_count_u32_elemens as well as
of_property_read_u32_index.
gpio-regulator change tested on a s3c2416-based device, ti-abb-regulator
compile-tested only.
[0] https://lkml.org/lkml/2014/1/16/172
Heiko Stuebner (3):
of: add functions to count number of elements in a property
regulator: gpio-regulator: do not open-code counting and access of dt array elements
regulator: ti-abb-regulator: do not open-code counting and access of dt array elements
drivers/of/base.c | 32 ++++++++++++++
drivers/regulator/gpio-regulator.c | 15 +++----
drivers/regulator/ti-abb-regulator.c | 43 +++++++++----------
include/linux/of.h | 76 ++++++++++++++++++++++++++++++++++
4 files changed, 134 insertions(+), 32 deletions(-)
--
1.7.10.4
^ permalink raw reply
* [PATCH 1/3] of: add functions to count number of elements in a property
From: Heiko Stübner @ 2014-02-12 0:00 UTC (permalink / raw)
To: grant.likely-QSEj5FYQhm4dnm+yROfE0A
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Liam Girdwood, Mark Brown
In-Reply-To: <2573561.oCYNnnL0gm@phil>
From: Heiko Stuebner <heiko.stuebner-HCpLIkUQxWGakBO8gow8eQ@public.gmane.org>
The need to know the number of array elements in a property is
a common pattern. To prevent duplication of open-coded implementations
add a helper static function that also centralises strict sanity
checking and DTB format details, as well as a set of wrapper functions
for u8, u16, u32 and u64.
Suggested-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: Heiko Stuebner <heiko.stuebner-HCpLIkUQxWGakBO8gow8eQ@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Acked-by: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
drivers/of/base.c | 32 ++++++++++++++++++++++
include/linux/of.h | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 108 insertions(+)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 239e3da..bed63c2 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -886,6 +886,38 @@ struct device_node *of_find_node_by_phandle(phandle handle)
EXPORT_SYMBOL(of_find_node_by_phandle);
/**
+ * of_property_count_elems_of_size - Count the number of elements in a property
+ *
+ * @np: device node from which the property value is to be read.
+ * @propname: name of the property to be searched.
+ * @elem_size: size of the individual element
+ *
+ * Search for a property in a device node and count the number of elements of
+ * size elem_size in it. Returns number of elements on sucess, -EINVAL if the
+ * property does not exist or its length does not match a multiple of elem_size
+ * and -ENODATA if the property does not have a value.
+ */
+int of_property_count_elems_of_size(const struct device_node *np,
+ const char *propname, int elem_size)
+{
+ struct property *prop = of_find_property(np, propname, NULL);
+
+ if (!prop)
+ return -EINVAL;
+ if (!prop->value)
+ return -ENODATA;
+
+ if (prop->length % elem_size != 0) {
+ pr_err("size of %s in node %s is not a multiple of %d\n",
+ propname, np->full_name, elem_size);
+ return -EINVAL;
+ }
+
+ return prop->length / elem_size;
+}
+EXPORT_SYMBOL_GPL(of_property_count_elems_of_size);
+
+/**
* of_find_property_value_of_size
*
* @np: device node from which the property value is to be read.
diff --git a/include/linux/of.h b/include/linux/of.h
index b3d0f6d..8cc9cef 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -218,6 +218,8 @@ extern struct device_node *of_find_node_with_property(
extern struct property *of_find_property(const struct device_node *np,
const char *name,
int *lenp);
+extern int of_property_count_elems_of_size(const struct device_node *np,
+ const char *propname, int elem_size);
extern int of_property_read_u32_index(const struct device_node *np,
const char *propname,
u32 index, u32 *out_value);
@@ -410,6 +412,12 @@ static inline struct device_node *of_find_compatible_node(
return NULL;
}
+static inline int of_property_count_elems_of_size(const struct device_node *np,
+ const char *propname, int elem_size)
+{
+ return -ENOSYS;
+}
+
static inline int of_property_read_u32_index(const struct device_node *np,
const char *propname, u32 index, u32 *out_value)
{
@@ -556,6 +564,74 @@ static inline struct device_node *of_find_matching_node(
}
/**
+ * of_property_count_u8_elems - Count the number of u8 elements in a property
+ *
+ * @np: device node from which the property value is to be read.
+ * @propname: name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u8 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u8 and -ENODATA if the
+ * property does not have a value.
+ */
+static inline int of_property_count_u8_elems(const struct device_node *np,
+ const char *propname)
+{
+ return of_property_count_elems_of_size(np, propname, sizeof(u8));
+}
+
+/**
+ * of_property_count_u16_elems - Count the number of u16 elements in a property
+ *
+ * @np: device node from which the property value is to be read.
+ * @propname: name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u16 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u16 and -ENODATA if the
+ * property does not have a value.
+ */
+static inline int of_property_count_u16_elems(const struct device_node *np,
+ const char *propname)
+{
+ return of_property_count_elems_of_size(np, propname, sizeof(u16));
+}
+
+/**
+ * of_property_count_u32_elems - Count the number of u32 elements in a property
+ *
+ * @np: device node from which the property value is to be read.
+ * @propname: name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u32 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u32 and -ENODATA if the
+ * property does not have a value.
+ */
+static inline int of_property_count_u32_elems(const struct device_node *np,
+ const char *propname)
+{
+ return of_property_count_elems_of_size(np, propname, sizeof(u32));
+}
+
+/**
+ * of_property_count_u64_elems - Count the number of u64 elements in a property
+ *
+ * @np: device node from which the property value is to be read.
+ * @propname: name of the property to be searched.
+ *
+ * Search for a property in a device node and count the number of u64 elements
+ * in it. Returns number of elements on sucess, -EINVAL if the property does
+ * not exist or its length does not match a multiple of u64 and -ENODATA if the
+ * property does not have a value.
+ */
+static inline int of_property_count_u64_elems(const struct device_node *np,
+ const char *propname)
+{
+ return of_property_count_elems_of_size(np, propname, sizeof(u64));
+}
+
+/**
* of_property_read_bool - Findfrom a property
* @np: device node from which the property value is to be read.
* @propname: name of the property to be searched.
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 2/3] regulator: gpio-regulator: do not open-code counting and access of dt array elements
From: Heiko Stübner @ 2014-02-12 0:01 UTC (permalink / raw)
To: grant.likely-QSEj5FYQhm4dnm+yROfE0A
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Liam Girdwood, Mark Brown
In-Reply-To: <2573561.oCYNnnL0gm@phil>
From: Heiko Stuebner <heiko.stuebner-HCpLIkUQxWGakBO8gow8eQ@public.gmane.org>
Open coding the counting of elements in a dt-property is abstracted by the newly
introduced of_property_count_uXX_elems functions. Additionally the raw iteration
over the states element exposes the endian conversion and dtb-format details,
which according to Mark Rutland "would be nice to limit [...] to of_ helper
functions".
Thus change gpio-regulator to use the helper for element counting and
of_property_read_u32_index for retrieval of individual values.
This makes it possible to remove the raw access to the states property entirely.
Signed-off-by: Heiko Stuebner <heiko.stuebner-HCpLIkUQxWGakBO8gow8eQ@public.gmane.org>
---
drivers/regulator/gpio-regulator.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/regulator/gpio-regulator.c b/drivers/regulator/gpio-regulator.c
index ac3a8c7..500aa3b 100644
--- a/drivers/regulator/gpio-regulator.c
+++ b/drivers/regulator/gpio-regulator.c
@@ -136,7 +136,6 @@ static struct gpio_regulator_config *
of_get_gpio_regulator_config(struct device *dev, struct device_node *np)
{
struct gpio_regulator_config *config;
- struct property *prop;
const char *regtype;
int proplen, gpio, i;
int ret;
@@ -191,14 +190,12 @@ of_get_gpio_regulator_config(struct device *dev, struct device_node *np)
}
/* Fetch states. */
- prop = of_find_property(np, "states", NULL);
- if (!prop) {
+ proplen = of_property_count_u32_elems(np, "states");
+ if (proplen < 0) {
dev_err(dev, "No 'states' property found\n");
return ERR_PTR(-EINVAL);
}
- proplen = prop->length / sizeof(int);
-
config->states = devm_kzalloc(dev,
sizeof(struct gpio_regulator_state)
* (proplen / 2),
@@ -207,10 +204,10 @@ of_get_gpio_regulator_config(struct device *dev, struct device_node *np)
return ERR_PTR(-ENOMEM);
for (i = 0; i < proplen / 2; i++) {
- config->states[i].value =
- be32_to_cpup((int *)prop->value + (i * 2));
- config->states[i].gpios =
- be32_to_cpup((int *)prop->value + (i * 2 + 1));
+ of_property_read_u32_index(np, "states", i * 2,
+ &config->states[i].value);
+ of_property_read_u32_index(np, "states", i * 2 + 1,
+ &config->states[i].gpios);
}
config->nr_states = i;
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 3/3] regulator: ti-abb-regulator: do not open-code counting and access of dt array elements
From: Heiko Stübner @ 2014-02-12 0:01 UTC (permalink / raw)
To: grant.likely-QSEj5FYQhm4dnm+yROfE0A
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Liam Girdwood, Mark Brown
In-Reply-To: <2573561.oCYNnnL0gm@phil>
From: Heiko Stuebner <heiko.stuebner-HCpLIkUQxWGakBO8gow8eQ@public.gmane.org>
Open coding the counting of elements in a dt-property is abstracted by the newly
introduced of_property_count_uXX_elems functions. Additionally the raw iteration
over the states element exposes the endian conversion and dtb-format details,
which according to Mark Rutland "would be nice to limit [...] to of_ helper
functions".
Thus change ti-abb-regulator to use the helper for element counting and
of_property_read_u32_index for retrieval of individual values.
This makes it possible to remove the raw access to the property entirely.
Signed-off-by: Heiko Stuebner <heiko.stuebner-HCpLIkUQxWGakBO8gow8eQ@public.gmane.org>
---
drivers/regulator/ti-abb-regulator.c | 43 ++++++++++++++++------------------
1 file changed, 20 insertions(+), 23 deletions(-)
diff --git a/drivers/regulator/ti-abb-regulator.c b/drivers/regulator/ti-abb-regulator.c
index a97d0c5..804c83a 100644
--- a/drivers/regulator/ti-abb-regulator.c
+++ b/drivers/regulator/ti-abb-regulator.c
@@ -507,32 +507,24 @@ static int ti_abb_init_table(struct device *dev, struct ti_abb *abb,
struct regulator_init_data *rinit_data)
{
struct ti_abb_info *info;
- const struct property *prop;
- const __be32 *abb_info;
const u32 num_values = 6;
char *pname = "ti,abb_info";
- u32 num_entries, i;
+ u32 i;
unsigned int *volt_table;
- int min_uV = INT_MAX, max_uV = 0;
+ int num_entries, min_uV = INT_MAX, max_uV = 0;
struct regulation_constraints *c = &rinit_data->constraints;
- prop = of_find_property(dev->of_node, pname, NULL);
- if (!prop) {
- dev_err(dev, "No '%s' property?\n", pname);
- return -ENODEV;
- }
-
- if (!prop->value) {
- dev_err(dev, "Empty '%s' property?\n", pname);
- return -ENODATA;
- }
-
/*
* Each abb_info is a set of n-tuple, where n is num_values, consisting
* of voltage and a set of detection logic for ABB information for that
* voltage to apply.
*/
- num_entries = prop->length / sizeof(u32);
+ num_entries = of_property_count_u32_elems(dev->of_node, pname);
+ if (num_entries < 0) {
+ dev_err(dev, "No '%s' property?\n", pname);
+ return -ENODEV;
+ }
+
if (!num_entries || (num_entries % num_values)) {
dev_err(dev, "All '%s' list entries need %d vals\n", pname,
num_values);
@@ -561,18 +553,23 @@ static int ti_abb_init_table(struct device *dev, struct ti_abb *abb,
/* We do not know where the OPP voltage is at the moment */
abb->current_info_idx = -EINVAL;
- abb_info = prop->value;
for (i = 0; i < num_entries; i++, info++, volt_table++) {
u32 efuse_offset, rbb_mask, fbb_mask, vset_mask;
u32 efuse_val;
/* NOTE: num_values should equal to entries picked up here */
- *volt_table = be32_to_cpup(abb_info++);
- info->opp_sel = be32_to_cpup(abb_info++);
- efuse_offset = be32_to_cpup(abb_info++);
- rbb_mask = be32_to_cpup(abb_info++);
- fbb_mask = be32_to_cpup(abb_info++);
- vset_mask = be32_to_cpup(abb_info++);
+ of_property_read_u32_index(dev->of_node, pname, i * num_values,
+ volt_table);
+ of_property_read_u32_index(dev->of_node, pname,
+ i * num_values + 1, &info->opp_sel);
+ of_property_read_u32_index(dev->of_node, pname,
+ i * num_values + 2, &efuse_offset);
+ of_property_read_u32_index(dev->of_node, pname,
+ i * num_values + 3, &rbb_mask);
+ of_property_read_u32_index(dev->of_node, pname,
+ i * num_values + 4, &fbb_mask);
+ of_property_read_u32_index(dev->of_node, pname,
+ i * num_values + 5, &vset_mask);
dev_dbg(dev,
"[%d]v=%d ABB=%d ef=0x%x rbb=0x%x fbb=0x%x vset=0x%x\n",
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: Linux-3.14-rc2: Order of serial node compatibles in DTS files.
From: Stephen N Chivers @ 2014-02-12 0:21 UTC (permalink / raw)
To: Sebastian Hesselbarth
Cc: Arnd Bergmann, Chris Proctor, devicetree, Kumar Gala,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, Stephen N Chivers,
Scott Wood
In-Reply-To: <52FAB65C.4090201-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote on
02/12/2014 10:46:36 AM:
> From: Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> To: Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Cc: Kumar Gala <galak-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>, Stephen N Chivers
> <schivers-znpAAEhiOVUQrrorzV6ljw@public.gmane.org>, Chris Proctor <cproctor-znpAAEhiOVUQrrorzV6ljw@public.gmane.org>,
> linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
> devicetree <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> Date: 02/12/2014 11:04 AM
> Subject: Re: Linux-3.14-rc2: Order of serial node compatibles in DTS
files.
>
> On 02/12/2014 12:41 AM, Scott Wood wrote:
> > On Tue, 2014-02-11 at 23:51 +0100, Sebastian Hesselbarth wrote:
> >> On 02/11/2014 11:33 PM, Kumar Gala wrote:
> >>> Hmm,
> >>>
> >>> Wondering if this caused the issue:
> >>>
> >>> commit 105353145eafb3ea919f5cdeb652a9d8f270228e
> >>> Author: Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>> Date: Tue Dec 3 14:52:00 2013 +0100
> >>>
> >>> OF: base: match each node compatible against all given matches
first
> >>
> >> [adding Arnd on Cc]
> >>
> >> Could be. I checked tty/serial/of_serial.c and it does not provide a
> >> compatible for "fsl,ns16550". Does reverting the patch fix the issue
> >> observed?
> >>
> >> I don't think the missing compatible is causing it, but of_serial
> >> provides a DT match for .type = "serial" just to fail later on
> >> with the error seen above.
> >>
> >> The commit in question reorders of_match_device in a way that match
> >> table order is not relevant anymore. This can cause it to match
> >> .type = "serial" first here.
> >>
> >> Rather than touching the commit, I suggest to remove the problematic
> >> .type = "serial" from the match table. It is of no use anyway.
> >
> > Regardless of whether .type = "serial" gets removed, it seems wrong
for
> > of_match_node() to accept a .type-only match (or .name, or anything
else
> > that doesn't involve .compatible) before it accepts a compatible match
> > other than the first in the compatible property.
>
> Right, I thought about it and came to the same conclusion. I sent a
> patch a second ago to prefer .compatible != NULL matches over those
> with .compatible == NULL.
>
> Would be great if Stephen can re-test that. If it solves the issue, I
> can send a patch tomorrow.
Done.
But, the Interrupt Controller (MPIC)
goes AWOL and it is down hill from there.
The MPIC is specified in the DTS as:
mpic: pic@40000 {
interrupt-controller;
#address-cells = <0>;
#interrupt-cells = <2>;
reg = <0x40000 0x40000>;
compatible = "chrp,open-pic";
device_type = "open-pic";
big-endian;
};
The board support file has the standard mechanism for allocating
the PIC:
struct mpic *mpic;
mpic = mpic_alloc(NULL, 0, 0, 0, 256, " OpenPIC ");
BUG_ON(mpic == NULL);
mpic_init(mpic);
I checked for damage in applying the patch and it has applied
correctly.
Stephen Chivers,
CSC Australia Pty. Ltd.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] ARM: dts: imx6qdl-sabresd: Do not place regulator nodes under simple-bus
From: Fabio Estevam @ 2014-02-12 0:31 UTC (permalink / raw)
To: shawn.guo-QSEj5FYQhm4dnm+yROfE0A
Cc: mark.rutland-5wv7dgnIgG8,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Fabio Estevam
From: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
According to Documentation/devicetree/bindings/regulator/regulator.txt
regulator nodes should not be placed under 'simple-bus'.
Mark Rutland also explains about it at:
http://www.spinics.net/lists/linux-usb/msg101497.html
Signed-off-by: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
Shawn,
I can convert other dts files if you are fine with this approach.
arch/arm/boot/dts/imx6qdl-sabresd.dtsi | 51 ++++++++++++++--------------------
1 file changed, 21 insertions(+), 30 deletions(-)
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index 0d816d3..d7df5b2 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -18,38 +18,29 @@
reg = <0x10000000 0x40000000>;
};
- regulators {
- compatible = "simple-bus";
- #address-cells = <1>;
- #size-cells = <0>;
-
- reg_usb_otg_vbus: regulator@0 {
- compatible = "regulator-fixed";
- reg = <0>;
- regulator-name = "usb_otg_vbus";
- regulator-min-microvolt = <5000000>;
- regulator-max-microvolt = <5000000>;
- gpio = <&gpio3 22 0>;
- enable-active-high;
- };
+ reg_usb_otg_vbus: regulator@0 {
+ compatible = "regulator-fixed";
+ regulator-name = "usb_otg_vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ gpio = <&gpio3 22 0>;
+ enable-active-high;
+ };
- reg_usb_h1_vbus: regulator@1 {
- compatible = "regulator-fixed";
- reg = <1>;
- regulator-name = "usb_h1_vbus";
- regulator-min-microvolt = <5000000>;
- regulator-max-microvolt = <5000000>;
- gpio = <&gpio1 29 0>;
- enable-active-high;
- };
+ reg_usb_h1_vbus: regulator@1 {
+ compatible = "regulator-fixed";
+ regulator-name = "usb_h1_vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ gpio = <&gpio1 29 0>;
+ enable-active-high;
+ };
- reg_audio: regulator@2 {
- compatible = "regulator-fixed";
- reg = <2>;
- regulator-name = "wm8962-supply";
- gpio = <&gpio4 10 0>;
- enable-active-high;
- };
+ reg_audio: regulator@2 {
+ compatible = "regulator-fixed";
+ regulator-name = "wm8962-supply";
+ gpio = <&gpio4 10 0>;
+ enable-active-high;
};
gpio-keys {
--
1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] Add a README file for dtc and libfdt
From: David Gibson @ 2014-02-12 0:35 UTC (permalink / raw)
To: Grant Likely
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, jdl-CYoMK+44s/E,
devicetree-compiler-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1392126992-8533-1-git-send-email-grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 402 bytes --]
On Tue, Feb 11, 2014 at 01:56:32PM +0000, Grant Likely wrote:
> Add a README file to document the location of the mailing list, the home
> page and state who the maintainers are.
Looks good, applied.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [RFC/PATCH 0/3] Add devicetree scanning for randomness
From: Laura Abbott @ 2014-02-12 1:33 UTC (permalink / raw)
To: Grant Likely, Rob Herring, Arnd Bergmann
Cc: Laura Abbott, linux-kernel, devicetree, linux-arm-kernel,
Kumar Gala, Kees Cook
Hi,
This is an RFC to seed the random number pool earlier when using devicetree.
The big issue this is trying to solve is the fact that the stack canary for
ARM tends to be the same across bootups of the same device. This is because
the random number pools do not get initialized until after the canary has
been set up. The canary can be moved later, but in general there is still
no way to reliably get random numbers early for other features (e.g. vector
randomization).
The goal here is to allow devices to add to the random pools via
add_device_randomness or some other method of their chosing at FDT time.
I realize that ARCH_RANDOM is already available but this didn't work because
1) ARCH_RANDOM is not multi-platform compatible without added
infrastructure to ARM
2) There is no uniform way to generate the random numbers which
would require an additional framework (same result as #1, different
reasons)
3) Given there is no uniform way to generate the random numbers, it seems
unlikely that 'early' random numbers would work the same as 'non-early'
random number which adds another layer of complexity.
The big reason to skip ARCH_RANDOM though is that the random number generation
we have would be reasonable if only seeded earlier.
Thanks,
Laura
Laura Abbott (3):
of: Add early randomness hooks
arm: Add ARCH_WANT_OF_RANDOMNESS
init: Move stack canary initialization after setup_arch
arch/arm/Kconfig | 3 ++
arch/arm/kernel/vmlinux.lds.S | 1 +
drivers/of/Kconfig | 7 ++++++
drivers/of/Makefile | 1 +
drivers/of/fdt.c | 7 ++++++
drivers/of/of_randomness.c | 43 +++++++++++++++++++++++++++++++++++++
include/asm-generic/vmlinux.lds.h | 5 ++++
include/linux/of_randomness.h | 42 ++++++++++++++++++++++++++++++++++++
init/main.c | 9 +++----
9 files changed, 113 insertions(+), 5 deletions(-)
create mode 100644 drivers/of/of_randomness.c
create mode 100644 include/linux/of_randomness.h
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ 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