Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] of: base: add support to get machine model name
From: Rob Herring @ 2016-12-09 16:03 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Frank Rowand, linux-kernel@vger.kernel.org, Arnd Bergmann,
	devicetree@vger.kernel.org
In-Reply-To: <195d492b-c674-e096-4f84-d37ca5448db2@arm.com>

On Wed, Nov 23, 2016 at 4:25 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 22/11/16 21:35, Rob Herring wrote:
>>
>> On Tue, Nov 22, 2016 at 12:44 PM, Frank Rowand <frowand.list@gmail.com>
>> wrote:
>
>
> [...]
>
>>>
>>> This patch adds a function that leads to conflating the "model" property
>>> and the "compatible" property. This leads to opaque, confusing and
>>> unclear
>>> code where ever it is used.   I think it is not good for the device tree
>>> framework to contribute to writing unclear code.
>>>
>>> Further, only two of the proposed users of this new function appear to
>>> be proper usage.  I do not think that the small amount of reduced lines
>>> of code is a good trade off for the reduced code clarity and for the
>>> potential for future mis-use of this function.
>>>
>>> Can I convince you to revert this patch?
>>
>>
>> Yes, I will revert.

I looked at this again and the users. They are all informational, so
I'm not worried if a compatible string could be returned with this
change. The function returns the best name for the machine and having
consistency is a good thing.

I was considering not reverting (as I'd not yet gotten around to it),
but I'm still going to revert for the naming.

>>
>>> If not, will you accept a patch to change the function name to more
>>> clearly indicate what it does?  (One possible name would be
>>> of_model_or_1st_compatible().)
>>
>>
>> I took it as there's already the FDT equivalent function.
>
>
> Yes it was mainly for non of_flat_* replacement for
> of_flat_dt_get_machine_name

I would suggest just of_get_machine_name().

You might also add a fallback to return "unknown", and drop some of
the custom strings. I don't think anyone should care about the actual
string. However, it's an error to have a DT with no model or top level
compatible, so maybe a WARN.

Rob

^ permalink raw reply

* Re: [PATCH v2 1/2] devicetree: i2c-hid: Add Wacom digitizer + regulator support
From: Doug Anderson @ 2016-12-09 16:05 UTC (permalink / raw)
  To: Rob Herring
  Cc: Benjamin Tissoires, Brian Norris, Jiri Kosina, Caesar Wang,
	open list:ARM/Rockchip SoC..., linux-input@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Dmitry Torokhov, Mark Rutland
In-Reply-To: <CAL_JsqK8ruinQC0pzWLWDLVcTmPdn4ZpejWUAQ0UD7H2MhLyvA@mail.gmail.com>

Hi,

On Thu, Dec 8, 2016 at 8:01 AM, Rob Herring <robh@kernel.org> wrote:
>> Just my $0.02.  Feel free to ignore.
>>
>> One thought is that I would say that the need to power on the device
>> explicitly seems more like a board level difference and less like a
>> difference associated with a particular digitizer.  Said another way,
>> it seems likely there will be boards with a w9013 without explicit
>> control of the regulator in software and it seems like there will be
>> boards with other digitizers where suddenly a new board will come out
>> that needs explicit control of the regulator.
>
> Then either the regulator is optional or you don't say it is a w9013
> for that board. But if you do need to initialize the device and
> therefore know what type of device it is, then you need a compatible
> for the device. It's when things really vary by board that we add DT
> properties.
>
>> In this particular case I feel like we can draw a lot of parallels to
>> an SDIO bus.
>>
>> When you specify an SDIO bus you don't specify what kind of card will
>> be present, you just say "I've got an SDIO bus" and then the specific
>> device underneath is probed.  Here we've say "I've got an i2c
>> connection intended for HID" and then you probe for the HID device
>> that's on the connection.
>
> No, the soldered down devices require all sorts of extra non-SDIO
> connections and we do specify the device in those cases.

We have never specified the device on boards I've worked with.

On rk3288-veyron, for instance, we might have stuffed a Broadcom 4354
WiFi chip or a Marvell 8897 WiFi chip.  Some veyron boards have one
chip and some have the other.  ...and during bringup we even had some
of the exact same boards where half were stuffed with one chip and
half with the other.

Nothing in the device tree says which chip is stuffed.  In both cases
the board uses the same power on sequence for the WiFi chip.  Once
that is done, we dynamically probe which actual WiFi part is stuffed.

Certainly not all users that have these WiFi chips use the same power
on sequence.  I have certainly seen development boards for these chips
where you just insert them into a regular SD card slot.  This is a
more expensive solution because you need more logic on the board, but
it shows that the power on sequence is not associated with these
chips.


>> Also for an SDIO bus, you've possibly got a regulators / GPIOs /
>> resets that need to be controlled, but the specific details of these
>> regulator / GPIOs / resets are specific to a given board and not
>> necessarily a given SDIO device.
>
> It's both. The device defines what is needed and the specs to control
> them (active states of GPIOs, de/assertion times of resets, supply
> voltages, etc.). The board only determines what the connections are
> and if you can control them.

It's not always that simple.  The device says that it needs power and
resets to happen.  How power is provided and how resets happen is
awfully board specific.  As per above it is possible that the board
wouldn't need to be involved above is you want to spend more money /
power.

-Doug

^ permalink raw reply

* Re: [PATCH v2 1/2] devicetree: i2c-hid: Add Wacom digitizer + regulator support
From: Doug Anderson @ 2016-12-09 16:16 UTC (permalink / raw)
  To: Rob Herring
  Cc: Jiri Kosina, Dmitry Torokhov, Benjamin Tissoires, Brian Norris,
	Caesar Wang, open list:ARM/Rockchip SoC...,
	linux-input@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Mark Rutland
In-Reply-To: <CAL_JsqLpjhHeeKq3PmNToux1Rgkg0M84-dv9HGCOws223ima4w@mail.gmail.com>

Hi,

On Fri, Dec 9, 2016 at 7:01 AM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Dec 9, 2016 at 8:36 AM, Jiri Kosina <jikos@kernel.org> wrote:
>> On Thu, 8 Dec 2016, Rob Herring wrote:
>>
>>> > And if tomorrow there is Elan device that is drop-in compatible (same
>>> > connector, etc) with Wacom i2c-hid, will you ask for Elan-specific
>>> > binding? Atmel? Weida? They all need to be powered up ultimately.
>>>
>>> Yes, I will.
>>
>> What advantage does that bring?
>>
>>> That in no way means the OS driver has to know about each and every one.
>>> If they can all claim compatibility with Wacom (including power
>>> control), then they can have a Wacom compatible string too. Or you can
>>> just never tell me that there's a different manufacturer and I won't
>>> care as long you don't need different control. But soon as a device
>>> needs another power rail, GPIO or different timing, then you'd better
>>> have a new compatible string.
>>
>> Again, I simply don't understand what advantage does the aproach you are
>> trying to use bring.
>
> This is simply how DT works. HID-over-I2C devices are no different
> than any other I2C device or any other component. You are not special.

...but once you say that it's HID over I2C then it becomes probe-able,
right?  Said another way: we need to specify just enough to power the
device up properly and then we can ask it what kind of device it is
and then we can make quirk decisions based on that.

So specifying what kind of device it is in the device tree is somewhat
redundant and it also means that you make it needlessly difficult to
build a system with dual-sourced components.

One of the major points of probe-able connections is that you could
put anything you want there and the kernel _doesn't_ need to describe
it.  ...and the whole point of device tree (I thought) was to
specifically handle connections that _aren't_ probe-able.

For instance, if a board has a USB bus on it but you need to assert a
special reset or turn on a special regulator (besides vbus) before you
can probe the USB bus, you wouldn't think that the board should
specify exactly what device was stuffed in the connection, would you?


>> HID over I2C is a generic protocol.
>
> DT describes h/w, not protocols.

Isn't it a little of each, though?  When you say that there's a USB
port or an SDIO port or a serial port on the board, you're saying more
than just that there are 2, 4, or 8 wires coming out of the board.
You're saying that you're expecting to talk a certain protocol over
those wires.

-Doug

^ permalink raw reply

* Re: [PATCH] devicetree: bindings: Add vendor prefix for Andes Technology Corporation
From: Rob Herring @ 2016-12-09 16:20 UTC (permalink / raw)
  To: Greentime Hu
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1479798905-21671-1-git-send-email-green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Tue, Nov 22, 2016 at 03:15:05PM +0800, Greentime Hu wrote:
> Add vendor-prefix for Andes Technology Corporation
> 
> Signed-off-by: Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/vendor-prefixes.txt        |    1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2] of/irq: improve error report on irq discovery process failure
From: Rob Herring @ 2016-12-09 16:25 UTC (permalink / raw)
  To: Guilherme G. Piccoli
  Cc: linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linuxppc-dev,
	Mark Rutland, Benjamin Herrenschmidt, Frank Rowand, Marc Zyngier
In-Reply-To: <5845B980.2030609-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

On Mon, Dec 5, 2016 at 1:01 PM, Guilherme G. Piccoli
<gpiccoli-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> wrote:
> On 12/05/2016 12:28 PM, Rob Herring wrote:
>> On Mon, Dec 5, 2016 at 7:59 AM, Guilherme G. Piccoli
>> <gpiccoli-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> wrote:
>>> On PowerPC machines some PCI slots might not have level triggered
>>> interrupts capability (also know as level signaled interrupts),
>>> leading of_irq_parse_pci() to complain by presenting error messages
>>> on the kernel log - in this case, the properties "interrupt-map" and
>>> "interrupt-map-mask" are not present on device's node in the device
>>> tree.
>>>
>>> This patch introduces a different message for this specific case,
>>> and also reduces its level from error to warning. Besides, we warn
>>> (once) that possibly some PCI slots on the system have no level
>>> triggered interrupts available.
>>> We changed some error return codes too on function of_irq_parse_raw()
>>> in order other failure's cases can be presented in a more precise way.
>>>
>>> Before this patch, when an adapter was plugged in a slot without level
>>> interrupts capabilitiy on PowerPC, we saw a generic error message
>>> like this:
>>>
>>>     [54.239] pci 002d:70:00.0: of_irq_parse_pci() failed with rc=-22
>>>
>>> Now, with this applied, we see the following specific message:
>>>
>>>     [16.154] pci 0014:60:00.1: of_irq_parse_pci: no interrupt-map found,
>>>     INTx interrupts not available
>>>
>>> Finally, we standardize the error path in of_irq_parse_raw() by always
>>> taking the fail path instead of returning directly from the loop.
>>>
>>> Signed-off-by: Guilherme G. Piccoli <gpiccoli-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>>> ---
>>>
>>> v2:
>>>   * Changed function return code to always return negative values;
>>
>> Are you sure this is safe? This is tricky because of differing values
>> of NO_IRQ (0 or -1).
>
> Thanks Rob, but this is purely bad wording from myself. I'm sorry - I
> meant to say that I changed only my positive return code (that was
> suggested to be removed in the prior revision) to negative return code!
>
> So, I changed only code I added myself in v1 =)
>
>
>>
>>>   * Improved/simplified warning outputs;
>>>   * Changed some return codes and some error paths in of_irq_parse_raw()
>>> in order to be more precise/consistent;
>>
>> This too could have some side effects on callers.
>>
>> Not saying don't do these changes, just need some assurances this has
>> been considered.
>
> Thanks for your attention. I performed a quick investigation before
> changing this, all the places that use the return values are just
> getting "true/false" information from that, meaning they just are
> comparing to 0 basically. So change -EINVAL to -ENOENT wouldn't hurt any
> user of these return values, it'll only become more informative IMHO.
>
> Now, regarding the only error path that was changed: for some reason,
> this was the only place in which we didn't goto fail label in case of
> failure - it was added by a legacy commit from Ben, dated from 2006:
> 006b64de60 ("[POWERPC] Make OF irq map code detect more error cases").
> Then it was carried by Grant Likely's commit 7dc2e1134a ("of/irq: merge
> irq mapping code"), 6-year old commit.
> I wasn't able to imagine a scenario in which changing this would break
> something; I believe the change improve consistency, but I'd remove it
> if you or somebody else thinks it worth be removed.

Okay. It's a bit late for 4.10 now and want this to be in -next for a
while, so I'll queue it after the merge window.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] devicetree: bindings: Add vendor prefix for Oki
From: Rob Herring @ 2016-12-09 16:28 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480068945-29822-1-git-send-email-geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

On Fri, Nov 25, 2016 at 11:15:45AM +0100, Geert Uytterhoeven wrote:
> Already in use for "oki,ml86v7667".
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] drivers/of: fix missing pr_cont()s in of_print_phandle_args
From: Rob Herring @ 2016-12-09 16:32 UTC (permalink / raw)
  To: Marcin Nowakowski; +Cc: Frank Rowand, devicetree, linux-kernel
In-Reply-To: <1480579255-28446-1-git-send-email-marcin.nowakowski@imgtec.com>

On Thu, Dec 01, 2016 at 09:00:55AM +0100, Marcin Nowakowski wrote:
> Since the KERN_CONT changes, the current debug printks have a lot of
> empty lines making the log messages very hard to read.
> 
> Signed-off-by: Marcin Nowakowski <marcin.nowakowski@imgtec.com>
> ---
>  drivers/of/base.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Applied, thanks.

Rob

^ permalink raw reply

* Re: [PATCH v4] of: Fix issue where code would fall through to error case.
From: Rob Herring @ 2016-12-09 16:33 UTC (permalink / raw)
  To: Moritz Fischer
  Cc: devicetree, linux-kernel, frowand.list, pantelis.antoniou,
	Moritz Fischer
In-Reply-To: <1480659025-18836-1-git-send-email-moritz.fischer@ettus.com>

On Thu, Dec 01, 2016 at 10:10:25PM -0800, Moritz Fischer wrote:
> From: Moritz Fischer <mdf@kernel.org>
> 
> No longer fall through into the error case that prints out
> an error if no error (err = 0) occurred.
> 
> Fixes d9181b20a83(of: Add back an error message, restructured)
> Signed-off-by: Moritz Fischer <mdf@kernel.org>
> Reviewed-by: Frank Rowand <frank.rowand@am.sony.com>
> ---

Applied, thanks.

Rob

^ permalink raw reply

* RE: [RFC] New Device Tree property - "bonding"
From: Ramesh Shanmugasundaram @ 2016-12-09 16:45 UTC (permalink / raw)
  To: Rob Herring
  Cc: Laurent Pinchart, frowand.list@gmail.com, mark.rutland@arm.com,
	pantelis.antoniou@konsulko.com, Chris Paterson,
	Geert Uytterhoeven, laurent.pinchart+renesas@ideasonboard.com,
	devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	Maxime Ripard
In-Reply-To: <CAL_JsqKJcEmNUzOm-3j3FODkN1faXMAMmURRxfRpHfiGs_a+qA@mail.gmail.com>

Hello Rob,

> > >> On Monday 05 Dec 2016 09:57:32 Rob Herring wrote:
> > >> > On Mon, Dec 5, 2016 at 8:40 AM, Laurent Pinchart wrote:
> > >> > > On Monday 05 Dec 2016 08:18:34 Rob Herring wrote:
> > >> > >> On Fri, Nov 25, 2016 at 10:55 AM, Ramesh Shanmugasundaram wrote:
> > >> > >>> Hello DT maintainers,
> > >> > >>>
> > >> > >>> In one of the Renesas SoCs we have a device called DRIF
> > >> > >>> (Digital Radio
> > >> > >>> Interface) controller. A DRIF channel contains 4 external
> > >> > >>> pins
> > >> > >>> - SCK, SYNC, Data pins D0 & D1.
> > >> > >>>
> > >> > >>> Internally a DRIF channel is made up of two SPI slave devices
> > >> > >>> (also called sub-channels here) that share common CLK & SYNC
> > >> > >>> signals but have their own resource set. The DRIF channel can
> > >> > >>> have either one of the sub-channel active at a time or both.
> > >> > >>> When both sub-channels are active, they need to be managed
> > >> > >>> together as one device as they share same CLK & SYNC. We plan
> > >> > >>> to tie these two sub-channels together with a new property
> > >> > >>> called
> > "renesas,bonding".
> > >> > >>
> > >> > >> Is there no need to describe the master device? No GPIOs,
> > >> > >> regulators or other sideband controls needed? If that's never
> > >> > >> needed (which seems doubtful), then I would do something
> > >> > >> different here probably with the master device as a child of
> > >> > >> one DRIF and then phandles to master from the other DRIFs.
> > >> > >> Otherwise, this looks
> > >> fine to me.
> > >> > >
> > >> > > Here's a bit of background.
> > >> > >
> > >> > > The DRIF is an SPI receiver. It has three input pins, a clock
> > >> > > line, a data line and a sync signal. The device is designed to
> > >> > > be connected to a variety of data sources, usually plain SPI (1
> > >> > > data line), IIS (1 data
> > >> > > line) but also radio tuners that output I/Q data
> > >> > > (http://www.ni.com/tutorial/4805/en/) over two data lines.
> > >> > >
> > >> > > In the case of IQ each data sample is split in two I and Q
> > >> > > values (typically 16 to 20 bits each in this case), and the
> > >> > > values are transmitted serially over one data line each. The
> > >> > > synchronization and clock signals are common to both data
> > >> > > lines. The DRIF is optimized for this use case as the DRIF
> > >> > > instances in the SoC (each of them having independent clocks,
> > >> > > interrupts and control
> > >> > > registers) are grouped by two, and the two instances in a group
> > >> > > handle a single data line each but share the same clock and
> > >> > > sync
> > input.
> > >> > >
> > >> > > On the software side we need to group the I and Q values, which
> > >> > > are DMA'ed to memory by the two DRIF instances, and make them
> > >> > > available to userspace. The V4L2 API used here in SDR (Software
> > >> > > Defined Radio) mode supports such use cases and exposes a
> > >> > > single device node to userspace that allows control of the two
> > >> > > DRIF instances as a single device. To be able to implement this
> > >> > > we need kernel code to be aware of DRIF groups and, while
> > >> > > binding to the DRIF instances separately, expose only one V4L2
> > >> > > device to
> > userspace for each group.
> > >> > >
> > >> > > There's no master or slave instance from a hardware point of
> > >> > > view, but the two instances are not interchangeable as they
> > >> > > carry separate
> > >> information.
> > >> > > They must thus be identified at the driver level.
> > >> >
> > >> > By master, I meant the external master device that generates the
> > >> > IQ data, not which of the internal DRIF blocks is a master of the
> other.
> > >> > So back to my question, does the external master device need to
> > >> > be described? I worry the answer now for a simple case is no, but
> > >> > then later people are going to have cases needing to describe
> > >> > more. We need to answer this question first before we can decide
> > >> > what this binding should look like.
> > >>
> > >> Oh yes the external device certainly needs to be described. As it
> > >> is controlled through a separate, general-purpose I2C or SPI
> > >> controller, it should be a child node of that controller. The DRIF
> > >> handles the data interface only, not the control interface of the
> external device.
> > >
> > > Yes, as Laurent mentioned, the external master will be described
> > separately. The data interface with the master is described through
> > port nodes. E.g.
> > >
> > >         port {
> > >                 drif0_ep: endpoint {
> > >                      remote-endpoint = <&tuner_ep>;
> > >                 };
> > >         };
> > >
> > > Do we agree on this model please?
> >
> > Well, that's not complete as you should have both DRIF0 and DRIF1
> > having connections to the tuner. Then you can walk the graph and find
> > everything, and you then don't need the bonding property.
> 
> Assuming the third party tuner exposes it's two data lines as two
> endpoints, it seems possible with of_graph.h apis to walk through tuner
> end points and get the phandle of the other DRIF device. However, there
> are couple of points coming to mind.
> 
> - The ctrl pins shared between two DRIFs needs to be enabled in one of the
> DRIF device. Do we choose this device arbitrarily? Do we expose the CTRL
> signal properties (msb/lsb first, polarity etc) on both DRIF devices?
> Should we think about scalability?
> 
> - It mandates the third party tuner device to expose it's two data lines
> as two endpoints. It assumes that a single third party master device
> controls both the data lines coming to each DRIF device.
> 
> The bonding property looks a bit cleaner on these aspects because it
> describes only the DRIF device.

Shall we please conclude on the model? Are you happy with the use of "renesas,bonding" property if the concern is on pushing this as a generic property?

I would appreciate your feedback.

Thanks,
Ramesh

^ permalink raw reply

* [PATCH v3 0/5] Static memory controllers for the Aspeed SoC
From: Cédric Le Goater @ 2016-12-09 16:49 UTC (permalink / raw)
  To: linux-mtd
  Cc: Mark Rutland, Boris Brezillon, devicetree, Richard Weinberger,
	Marek Vasut, Rob Herring, Joel Stanley, Cyrille Pitchen,
	Brian Norris, David Woodhouse, Cédric Le Goater

Hello,

Here is a serie introducing a new driver for the different memory
controllers of the Aspeed AST2500 and AST2400 SoCs. Each SoC has at
least a memory controller for the BMC firmware and another one for the
host firmware. The register set are mostly comptatible but there are
some slight differences on the AST2400.

The driver only support SPI type flash.

Tested on:

 * OpenPOWER Palmetto (AST2400) with
 	FMC controller : n25q256a
	SPI controller : mx25l25635e and n25q512ax3

 * Evaluation board (AST2500) with
 	FMC controller : w25q256 
	SPI controller : w25q256

 * OpenPOWER Witherspoon (AST2500) with
 	FMC controller : mx25l25635e * 2
	SPI controller : mx66l1g45g

Changes since v2:
 - splitted patch to distinguish AST2400 and AST2500 controllers
 - fixed controller names
 - introduced prepare/unprepare ops
 - introduced a aspeed_smc_setup_flash() routine
 - various cleanups

Changes since v1:
 - added a set4b ops to handle difference in the controllers
 - simplified the IO routines
 - prepared for fast read using dummy cycles

Work in progress:
 - read optimization using higher SPI clock frequencies
 - command mode to direct reads from AHB
 - DMA support

Thanks,

C.

Cédric Le Goater (5):
  mtd: spi-nor: add memory controllers for the Aspeed AST2500 SoC
  mtd: aspeed: add memory controllers for the Aspeed AST2400 SoC
  mtd: aspeed: used a label property
  mtd: spi-nor: bindings for the Aspeed memory controllers
  mtd: spi-nor: add a label property to jedec,spi-nor

 .../devicetree/bindings/mtd/aspeed-smc.txt         |  51 ++
 .../devicetree/bindings/mtd/jedec,spi-nor.txt      |   2 +
 drivers/mtd/spi-nor/Kconfig                        |  10 +
 drivers/mtd/spi-nor/Makefile                       |   1 +
 drivers/mtd/spi-nor/aspeed-smc.c                   | 770 +++++++++++++++++++++
 5 files changed, 834 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/aspeed-smc.txt
 create mode 100644 drivers/mtd/spi-nor/aspeed-smc.c

-- 
2.7.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply

* [PATCH v3 1/5] mtd: spi-nor: add memory controllers for the Aspeed AST2500 SoC
From: Cédric Le Goater @ 2016-12-09 16:49 UTC (permalink / raw)
  To: linux-mtd
  Cc: Mark Rutland, Boris Brezillon, devicetree, Richard Weinberger,
	Marek Vasut, Rob Herring, Joel Stanley, Cyrille Pitchen,
	Brian Norris, David Woodhouse, Cédric Le Goater
In-Reply-To: <1481302167-28044-1-git-send-email-clg@kaod.org>

This driver adds mtd support for the Aspeed AST2500 SoC static memory
controllers :

 * Firmware SPI Memory Controller (FMC)
   . BMC firmware
   . 3 chip select pins (CE0 ~ CE2)
   . supports SPI type flash memory (CE0-CE1)
   . CE2 can be of NOR type flash but this is not supported by the
     driver

 * SPI Flash Controller (SPI1 and SPI2)
   . host firmware
   . 2 chip select pins (CE0 ~ CE1)
   . supports SPI type flash memory

Each controller has a memory range on which it maps its flash module
slaves. Each slave is assigned a memory window for its mapping that
can be changed at bootime with the Segment Address Register.

Each SPI flash slave can then be accessed in two modes: Command and
User. When in User mode, accesses to the memory segment of the slaves
are translated in SPI transfers. When in Command mode, the HW
generates the SPI commands automatically and the memory segment is
accessed as if doing a MMIO.

Currently, only the User mode is supported. Command mode needs a
little more work to check that the memory window on the AHB bus fits
the module size.

Based on previous work from Milton D. Miller II <miltonm@us.ibm.com>

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 drivers/mtd/spi-nor/Kconfig      |  10 +
 drivers/mtd/spi-nor/Makefile     |   1 +
 drivers/mtd/spi-nor/aspeed-smc.c | 735 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 746 insertions(+)
 create mode 100644 drivers/mtd/spi-nor/aspeed-smc.c

diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 4a682ee0f632..5c0efbd9dd89 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -29,6 +29,16 @@ config MTD_SPI_NOR_USE_4K_SECTORS
 	  Please note that some tools/drivers/filesystems may not work with
 	  4096 B erase size (e.g. UBIFS requires 15 KiB as a minimum).
 
+config SPI_ASPEED
+	tristate "Aspeed flash controllers in SPI mode"
+	depends on ARCH_ASPEED || COMPILE_TEST
+	depends on HAS_IOMEM && OF
+	help
+	  This enables support for the Firmware Memory controller (FMC)
+	  in the Aspeed AST2500 SoC when attached to SPI NOR chips,
+	  and support for the SPI flash memory controller (SPI) for
+	  the host firmware. The implementation only supports SPI NOR.
+
 config SPI_ATMEL_QUADSPI
 	tristate "Atmel Quad SPI Controller"
 	depends on ARCH_AT91 || (ARM && COMPILE_TEST)
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 121695e83542..d73d772c689c 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_MTD_SPI_NOR)	+= spi-nor.o
+obj-$(CONFIG_SPI_ASPEED)	+= aspeed-smc.o
 obj-$(CONFIG_SPI_ATMEL_QUADSPI)	+= atmel-quadspi.o
 obj-$(CONFIG_SPI_CADENCE_QUADSPI)	+= cadence-quadspi.o
 obj-$(CONFIG_SPI_FSL_QUADSPI)	+= fsl-quadspi.o
diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
new file mode 100644
index 000000000000..6f9244f07aef
--- /dev/null
+++ b/drivers/mtd/spi-nor/aspeed-smc.c
@@ -0,0 +1,735 @@
+/*
+ * ASPEED Static Memory Controller driver
+ *
+ * Copyright (c) 2015-2016, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/bug.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/spi-nor.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/sysfs.h>
+
+#define DEVICE_NAME	"aspeed-smc"
+
+/*
+ * In user mode all data bytes read or written to the chip decode address
+ * range are transferred to or from the SPI bus. The range is treated as a
+ * fifo of arbitratry 1, 2, or 4 byte width but each write has to be aligned
+ * to its size. The address within the multiple 8kB range is ignored when
+ * sending bytes to the SPI bus.
+ *
+ * On the arm architecture, as of Linux version 4.3, memcpy_fromio and
+ * memcpy_toio on little endian targets use the optimized memcpy routines
+ * that were designed for well behavied memory storage. These routines
+ * have a stutter if the source and destination are not both word aligned,
+ * once with a duplicate access to the source after aligning to the
+ * destination to a word boundary, and again with a duplicate access to
+ * the source when the final byte count is not word aligned.
+ *
+ * When writing or reading the fifo this stutter discards data or sends
+ * too much data to the fifo and can not be used by this driver.
+ *
+ * While the low level io string routines that implement the insl family do
+ * the desired accesses and memory increments, the cross architecture io
+ * macros make them essentially impossible to use on a memory mapped address
+ * instead of a a token from the call to iomap of an io port.
+ *
+ * These fifo routines use readl and friends to a constant io port and update
+ * the memory buffer pointer and count via explicit code. The final updates
+ * to len are optimistically suppressed.
+ */
+static int aspeed_smc_read_from_ahb(void *buf, const void __iomem *src,
+				    size_t len)
+{
+	if (IS_ALIGNED((u32)src, sizeof(u32)) &&
+	    IS_ALIGNED((u32)buf, sizeof(u32)) &&
+	    IS_ALIGNED(len, sizeof(u32))) {
+		while (len > 3) {
+			*(u32 *)buf = readl(src);
+			buf += 4;
+			src += 4;
+			len -= 4;
+		}
+	}
+
+	while (len--) {
+		*(u8 *)buf = readb(src);
+		buf += 1;
+		src += 1;
+	}
+	return 0;
+}
+
+static int aspeed_smc_write_to_ahb(void __iomem *dst, const void *buf,
+				   size_t len)
+{
+	if (IS_ALIGNED((u32)dst, sizeof(u32)) &&
+	    IS_ALIGNED((u32)buf, sizeof(u32)) &&
+	    IS_ALIGNED(len, sizeof(u32))) {
+		while (len > 3) {
+			u32 val = *(u32 *)buf;
+
+			writel(val, dst);
+			buf += 4;
+			dst += 4;
+			len -= 4;
+		}
+	}
+
+	while (len--) {
+		u8 val = *(u8 *)buf;
+
+		writeb(val, dst);
+		buf += 1;
+		dst += 1;
+	}
+	return 0;
+}
+
+/*
+ * The driver only support SPI flash
+ */
+enum aspeed_smc_flash_type {
+	smc_type_nor  = 0,
+	smc_type_nand = 1,
+	smc_type_spi  = 2,
+};
+
+struct aspeed_smc_chip;
+
+struct aspeed_smc_info {
+	u32 maxsize;		/* maximum size of chip window */
+	u8 nce;			/* number of chip enables */
+	bool hastype;		/* flash type field exists in config reg */
+	u8 we0;			/* shift for write enable bit for CE0 */
+	u8 ctl0;		/* offset in regs of ctl for CE0 */
+
+	void (*set_4b)(struct aspeed_smc_chip *chip);
+};
+
+static void aspeed_smc_chip_set_4b(struct aspeed_smc_chip *chip);
+
+static const struct aspeed_smc_info fmc_2500_info = {
+	.maxsize = 256 * 1024 * 1024,
+	.nce = 3,
+	.hastype = true,
+	.we0 = 16,
+	.ctl0 = 0x10,
+	.set_4b = aspeed_smc_chip_set_4b,
+};
+
+static const struct aspeed_smc_info spi_2500_info = {
+	.maxsize = 128 * 1024 * 1024,
+	.nce = 2,
+	.hastype = false,
+	.we0 = 16,
+	.ctl0 = 0x10,
+	.set_4b = aspeed_smc_chip_set_4b,
+};
+
+enum aspeed_smc_ctl_reg_value {
+	smc_base,		/* base value without mode for other commands */
+	smc_read,		/* command reg for (maybe fast) reads */
+	smc_write,		/* command reg for writes */
+	smc_max,
+};
+
+struct aspeed_smc_controller;
+
+struct aspeed_smc_chip {
+	int cs;
+	struct aspeed_smc_controller *controller;
+	void __iomem *ctl;			/* control register */
+	void __iomem *ahb_base;			/* base of chip window */
+	u32 ctl_val[smc_max];			/* control settings */
+	enum aspeed_smc_flash_type type;	/* what type of flash */
+	struct spi_nor nor;
+};
+
+struct aspeed_smc_controller {
+	struct device *dev;
+
+	struct mutex mutex;			/* controller access mutex */
+	const struct aspeed_smc_info *info;	/* type info of controller */
+	void __iomem *regs;			/* controller registers */
+	void __iomem *ahb_base;			/* per-chip windows resource */
+
+	struct aspeed_smc_chip *chips[0];	/* pointers to attached chips */
+};
+
+/*
+ * SPI Flash Configuration Register (AST2500 SPI)
+ *     or
+ * Type setting Register (AST2500 FMC).
+ * CE0 and CE1 can only be of type SPI. CE2 can be of type NOR but the
+ * driver does not support it.
+ */
+#define CONFIG_REG			0x0
+#define CONFIG_DISABLE_LEGACY		BIT(31) /* 1 */
+
+#define CONFIG_CE2_WRITE		BIT(18)
+#define CONFIG_CE1_WRITE		BIT(17)
+#define CONFIG_CE0_WRITE		BIT(16)
+
+#define CONFIG_CE2_TYPE			BIT(4) /* AST2500 FMC only */
+#define CONFIG_CE1_TYPE			BIT(2) /* AST2500 FMC only */
+#define CONFIG_CE0_TYPE			BIT(0) /* AST2500 FMC only */
+
+/*
+ * CE Control Register
+ */
+#define CE_CONTROL_REG			0x4
+
+/*
+ * CEx Control Register
+ */
+#define CONTROL_AAF_MODE		BIT(31)
+#define CONTROL_IO_MODE_MASK		GENMASK(30, 28)
+#define CONTROL_IO_DUAL_DATA		BIT(29)
+#define CONTROL_IO_DUAL_ADDR_DATA	(BIT(29) | BIT(28))
+#define CONTROL_IO_QUAD_DATA		BIT(30)
+#define CONTROL_IO_QUAD_ADDR_DATA	(BIT(30) | BIT(28))
+#define CONTROL_CE_INACTIVE_SHIFT	24
+#define CONTROL_CE_INACTIVE_MASK	GENMASK(27, \
+					CONTROL_CE_INACTIVE_SHIFT)
+/* 0 = 16T ... 15 = 1T   T=HCLK */
+#define CONTROL_COMMAND_SHIFT		16
+#define CONTROL_DUMMY_COMMAND_OUT	BIT(15)
+#define CONTROL_IO_DUMMY_HI		BIT(14)
+#define CONTROL_IO_DUMMY_HI_SHIFT	14
+#define CONTROL_CLK_DIV4		BIT(13) /* others */
+#define CONTROL_RW_MERGE		BIT(12)
+#define CONTROL_IO_DUMMY_LO_SHIFT	6
+#define CONTROL_IO_DUMMY_LO		GENMASK(7, \
+						CONTROL_IO_DUMMY_LO_SHIFT)
+#define CONTROL_IO_DUMMY_MASK		(CONTROL_IO_DUMMY_HI | \
+					 CONTROL_IO_DUMMY_LO)
+#define CONTROL_IO_DUMMY_SET(dummy)				 \
+	(((((dummy) >> 2) & 0x1) << CONTROL_IO_DUMMY_HI_SHIFT) | \
+	 (((dummy) & 0x3) << CONTROL_IO_DUMMY_LO_SHIFT))
+
+#define CONTROL_CLOCK_FREQ_SEL_SHIFT	8
+#define CONTROL_CLOCK_FREQ_SEL_MASK	GENMASK(11, \
+						CONTROL_CLOCK_FREQ_SEL_SHIFT)
+#define CONTROL_LSB_FIRST		BIT(5)
+#define CONTROL_CLOCK_MODE_3		BIT(4)
+#define CONTROL_IN_DUAL_DATA		BIT(3)
+#define CONTROL_CE_STOP_ACTIVE_CONTROL	BIT(2)
+#define CONTROL_COMMAND_MODE_MASK	GENMASK(1, 0)
+#define CONTROL_COMMAND_MODE_NORMAL	(0)
+#define CONTROL_COMMAND_MODE_FREAD	(1)
+#define CONTROL_COMMAND_MODE_WRITE	(2)
+#define CONTROL_COMMAND_MODE_USER	(3)
+
+#define CONTROL_KEEP_MASK						\
+	(CONTROL_AAF_MODE | CONTROL_CE_INACTIVE_MASK | CONTROL_CLK_DIV4 | \
+	 CONTROL_IO_DUMMY_MASK | CONTROL_CLOCK_FREQ_SEL_MASK |		\
+	 CONTROL_LSB_FIRST | CONTROL_CLOCK_MODE_3)
+
+/*
+ * Segment Address Registers. Start and end addresses are encoded
+ * using 8MB units
+ */
+#define SEGMENT_ADDR_REG0		0x30
+#define SEGMENT_ADDR_START(_r)		((((_r) >> 16) & 0xFF) << 23)
+#define SEGMENT_ADDR_END(_r)		((((_r) >> 24) & 0xFF) << 23)
+
+static inline u32 aspeed_smc_chip_write_bit(struct aspeed_smc_chip *chip)
+{
+	return BIT(chip->controller->info->we0 + chip->cs);
+}
+
+static void aspeed_smc_chip_check_config(struct aspeed_smc_chip *chip)
+{
+	struct aspeed_smc_controller *controller = chip->controller;
+	u32 reg;
+
+	reg = readl(controller->regs + CONFIG_REG);
+
+	if (reg & aspeed_smc_chip_write_bit(chip))
+		return;
+
+	dev_dbg(controller->dev, "config write is not set ! @%p: 0x%08x\n",
+		controller->regs + CONFIG_REG, reg);
+	reg |= aspeed_smc_chip_write_bit(chip);
+	writel(reg, controller->regs + CONFIG_REG);
+}
+
+static void aspeed_smc_start_user(struct spi_nor *nor)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+	u32 ctl = chip->ctl_val[smc_base];
+
+	/*
+	 * When the chip is controlled in user mode, we need write
+	 * access to send the opcodes to it. So check the config.
+	 */
+	aspeed_smc_chip_check_config(chip);
+
+	ctl |= CONTROL_COMMAND_MODE_USER |
+		CONTROL_CE_STOP_ACTIVE_CONTROL;
+	writel(ctl, chip->ctl);
+
+	ctl &= ~CONTROL_CE_STOP_ACTIVE_CONTROL;
+	writel(ctl, chip->ctl);
+}
+
+static void aspeed_smc_stop_user(struct spi_nor *nor)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	u32 ctl = chip->ctl_val[smc_read];
+	u32 ctl2 = ctl | CONTROL_COMMAND_MODE_USER |
+		CONTROL_CE_STOP_ACTIVE_CONTROL;
+
+	writel(ctl2, chip->ctl);	/* stop user CE control */
+	writel(ctl, chip->ctl);		/* default to fread or read mode */
+}
+
+static int aspeed_smc_prep(struct spi_nor *nor, enum spi_nor_ops ops)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	mutex_lock(&chip->controller->mutex);
+	return 0;
+}
+
+static void aspeed_smc_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	mutex_unlock(&chip->controller->mutex);
+}
+
+static int aspeed_smc_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	aspeed_smc_start_user(nor);
+	aspeed_smc_write_to_ahb(chip->ahb_base, &opcode, 1);
+	aspeed_smc_read_from_ahb(buf, chip->ahb_base, len);
+	aspeed_smc_stop_user(nor);
+	return 0;
+}
+
+static int aspeed_smc_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf,
+				int len)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	aspeed_smc_start_user(nor);
+	aspeed_smc_write_to_ahb(chip->ahb_base, &opcode, 1);
+	aspeed_smc_write_to_ahb(chip->ahb_base, buf, len);
+	aspeed_smc_stop_user(nor);
+	return 0;
+}
+
+static void aspeed_smc_send_cmd_addr(struct spi_nor *nor, u8 cmd, u32 addr)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+	__be32 temp;
+	u32 cmdaddr;
+
+	switch (nor->addr_width) {
+	default:
+		WARN_ONCE(1, "Unexpected address width %u, defaulting to 3\n",
+			  nor->addr_width);
+		/* FALLTHROUGH */
+	case 3:
+		cmdaddr = addr & 0xFFFFFF;
+		cmdaddr |= cmd << 24;
+
+		temp = cpu_to_be32(cmdaddr);
+		aspeed_smc_write_to_ahb(chip->ahb_base, &temp, 4);
+		break;
+	case 4:
+		temp = cpu_to_be32(addr);
+		aspeed_smc_write_to_ahb(chip->ahb_base, &cmd, 1);
+		aspeed_smc_write_to_ahb(chip->ahb_base, &temp, 4);
+		break;
+	}
+}
+
+static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from,
+				    size_t len, u_char *read_buf)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	aspeed_smc_start_user(nor);
+	aspeed_smc_send_cmd_addr(nor, nor->read_opcode, from);
+	aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len);
+	aspeed_smc_stop_user(nor);
+	return len;
+}
+
+static ssize_t aspeed_smc_write_user(struct spi_nor *nor, loff_t to,
+				     size_t len, const u_char *write_buf)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	aspeed_smc_start_user(nor);
+	aspeed_smc_send_cmd_addr(nor, nor->program_opcode, to);
+	aspeed_smc_write_to_ahb(chip->ahb_base, write_buf, len);
+	aspeed_smc_stop_user(nor);
+	return len;
+}
+
+static int aspeed_smc_unregister(struct aspeed_smc_controller *controller)
+{
+	struct aspeed_smc_chip *chip;
+	int n;
+
+	for (n = 0; n < controller->info->nce; n++) {
+		chip = controller->chips[n];
+		if (chip)
+			mtd_device_unregister(&chip->nor.mtd);
+	}
+
+	return 0;
+}
+
+static int aspeed_smc_remove(struct platform_device *dev)
+{
+	return aspeed_smc_unregister(platform_get_drvdata(dev));
+}
+
+static const struct of_device_id aspeed_smc_matches[] = {
+	{ .compatible = "aspeed,ast2500-fmc", .data = &fmc_2500_info },
+	{ .compatible = "aspeed,ast2500-spi", .data = &spi_2500_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, aspeed_smc_matches);
+
+/*
+ * Each chip has a mapping window defined by a segment address
+ * register defining a start and an end address on the AHB bus. These
+ * addresses can be configured to fit the chip size and offer a
+ * contiguous memory region across chips. For the moment, we only
+ * check that each chip segment is valid.
+ */
+static void __iomem *aspeed_smc_chip_base(struct aspeed_smc_chip *chip,
+					  struct resource *res)
+{
+	struct aspeed_smc_controller *controller = chip->controller;
+	u32 offset = 0;
+	u32 reg;
+
+	if (controller->info->nce > 1) {
+		reg = readl(controller->regs + SEGMENT_ADDR_REG0 +
+			    chip->cs * 4);
+
+		if (SEGMENT_ADDR_START(reg) >= SEGMENT_ADDR_END(reg))
+			return NULL;
+
+		offset = SEGMENT_ADDR_START(reg) - res->start;
+	}
+
+	return controller->ahb_base + offset;
+}
+
+static void aspeed_smc_chip_enable_write(struct aspeed_smc_chip *chip)
+{
+	struct aspeed_smc_controller *controller = chip->controller;
+	u32 reg;
+
+	reg = readl(controller->regs + CONFIG_REG);
+
+	reg |= aspeed_smc_chip_write_bit(chip);
+	writel(reg, controller->regs + CONFIG_REG);
+}
+
+static void aspeed_smc_chip_set_type(struct aspeed_smc_chip *chip, int type)
+{
+	struct aspeed_smc_controller *controller = chip->controller;
+	u32 reg;
+
+	chip->type = type;
+
+	reg = readl(controller->regs + CONFIG_REG);
+	reg &= ~(3 << (chip->cs * 2));
+	reg |= chip->type << (chip->cs * 2);
+	writel(reg, controller->regs + CONFIG_REG);
+}
+
+/*
+ * The AST2500 FMC flash controller should be strapped by hardware, or
+ * autodetected, but the AST2500 SPI flash needs to be set.
+ */
+static void aspeed_smc_chip_set_4b(struct aspeed_smc_chip *chip)
+{
+	struct aspeed_smc_controller *controller = chip->controller;
+	u32 reg;
+
+	if (chip->controller->info == &spi_2500_info) {
+		reg = readl(controller->regs + CE_CONTROL_REG);
+		reg |= 1 << chip->cs;
+		writel(reg, controller->regs + CE_CONTROL_REG);
+	}
+}
+
+static int aspeed_smc_chip_setup_init(struct aspeed_smc_chip *chip,
+				      struct resource *res)
+{
+	struct aspeed_smc_controller *controller = chip->controller;
+	const struct aspeed_smc_info *info = controller->info;
+	u32 reg, base_reg;
+
+	/*
+	 * Always turn on the write enable bit to allow opcodes to be
+	 * sent in user mode.
+	 */
+	aspeed_smc_chip_enable_write(chip);
+
+	/* The driver only supports SPI type flash */
+	if (info->hastype)
+		aspeed_smc_chip_set_type(chip, smc_type_spi);
+
+	/*
+	 * Configure chip base address in memory
+	 */
+	chip->ahb_base = aspeed_smc_chip_base(chip, res);
+	if (!chip->ahb_base) {
+		dev_warn(chip->nor.dev, "CE segment window closed.\n");
+		return -1;
+	}
+
+	/*
+	 * Get value of the inherited control register. U-Boot usually
+	 * does some timing calibration on the FMC chip, so it's good
+	 * to keep them. In the future, we should handle calibration
+	 * from Linux.
+	 */
+	reg = readl(chip->ctl);
+	dev_dbg(controller->dev, "control register: %08x\n", reg);
+
+	base_reg = reg & CONTROL_KEEP_MASK;
+	if (base_reg != reg) {
+		dev_info(controller->dev,
+			 "control register changed to: %08x\n",
+			 base_reg);
+	}
+	chip->ctl_val[smc_base] = base_reg;
+
+	/*
+	 * Retain the prior value of the control register as the
+	 * default if it was normal access mode. Otherwise start with
+	 * the sanitized base value set to read mode.
+	 */
+	if ((reg & CONTROL_COMMAND_MODE_MASK) ==
+	    CONTROL_COMMAND_MODE_NORMAL)
+		chip->ctl_val[smc_read] = reg;
+	else
+		chip->ctl_val[smc_read] = chip->ctl_val[smc_base] |
+			CONTROL_COMMAND_MODE_NORMAL;
+
+	dev_dbg(controller->dev, "default control register: %08x\n",
+		chip->ctl_val[smc_read]);
+	return 0;
+}
+
+static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip)
+{
+	struct aspeed_smc_controller *controller = chip->controller;
+	const struct aspeed_smc_info *info = controller->info;
+	u32 cmd;
+
+	if (chip->nor.addr_width == 4 && info->set_4b)
+		info->set_4b(chip);
+
+	/*
+	 * base mode has not been optimized yet. use it for writes.
+	 */
+	chip->ctl_val[smc_write] = chip->ctl_val[smc_base] |
+		chip->nor.program_opcode << CONTROL_COMMAND_SHIFT |
+		CONTROL_COMMAND_MODE_WRITE;
+
+	dev_dbg(controller->dev, "write control register: %08x\n",
+		chip->ctl_val[smc_write]);
+
+	/*
+	 * TODO: Adjust clocks if fast read is supported and interpret
+	 * SPI-NOR flags to adjust controller settings.
+	 */
+	switch (chip->nor.flash_read) {
+	case SPI_NOR_NORMAL:
+		cmd = CONTROL_COMMAND_MODE_NORMAL;
+		break;
+	case SPI_NOR_FAST:
+		cmd = CONTROL_COMMAND_MODE_FREAD;
+		break;
+	default:
+		dev_err(chip->nor.dev, "unsupported SPI read mode\n");
+		return -EINVAL;
+	}
+
+	chip->ctl_val[smc_read] |= cmd |
+		CONTROL_IO_DUMMY_SET(chip->nor.read_dummy / 8);
+
+	dev_dbg(controller->dev, "base control register: %08x\n",
+		chip->ctl_val[smc_read]);
+	return 0;
+}
+
+static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
+				  struct device_node *np, struct resource *r)
+{
+	const struct aspeed_smc_info *info = controller->info;
+	struct device *dev = controller->dev;
+	struct device_node *child;
+	unsigned int cs;
+	int ret = -ENODEV;
+
+	for_each_available_child_of_node(np, child) {
+		struct aspeed_smc_chip *chip;
+		struct spi_nor *nor;
+		struct mtd_info *mtd;
+
+		/* This driver does not support NAND or NOR flash devices. */
+		if (!of_device_is_compatible(child, "jedec,spi-nor"))
+			continue;
+
+		ret = of_property_read_u32(child, "reg", &cs);
+		if (ret) {
+			dev_err(dev, "Couldn't not read chip select.\n");
+			break;
+		}
+
+		if (cs >= info->nce) {
+			dev_err(dev, "Chip select %d out of range.\n",
+				cs);
+			ret = -ERANGE;
+			break;
+		}
+
+		if (controller->chips[cs]) {
+			dev_err(dev, "Chip select %d already in use by %s\n",
+				cs, dev_name(controller->chips[cs]->nor.dev));
+			ret = -EBUSY;
+			break;
+		}
+
+		chip = devm_kzalloc(controller->dev, sizeof(*chip), GFP_KERNEL);
+		if (!chip) {
+			ret = -ENOMEM;
+			break;
+		}
+
+		chip->controller = controller;
+		chip->ctl = controller->regs + info->ctl0 + cs * 4;
+		chip->cs = cs;
+
+		nor = &chip->nor;
+		mtd = &nor->mtd;
+
+		nor->dev = dev;
+		nor->priv = chip;
+		spi_nor_set_flash_node(nor, child);
+		nor->read = aspeed_smc_read_user;
+		nor->write = aspeed_smc_write_user;
+		nor->read_reg = aspeed_smc_read_reg;
+		nor->write_reg = aspeed_smc_write_reg;
+		nor->prepare = aspeed_smc_prep;
+		nor->unprepare = aspeed_smc_unprep;
+
+		ret = aspeed_smc_chip_setup_init(chip, r);
+		if (ret)
+			break;
+
+		/*
+		 * TODO: Add support for SPI_NOR_QUAD and SPI_NOR_DUAL
+		 * attach when board support is present as determined
+		 * by of property.
+		 */
+		ret = spi_nor_scan(nor, NULL, SPI_NOR_NORMAL);
+		if (ret)
+			break;
+
+		ret = aspeed_smc_chip_setup_finish(chip);
+		if (ret)
+			break;
+
+		ret = mtd_device_register(mtd, NULL, 0);
+		if (ret)
+			break;
+
+		controller->chips[cs] = chip;
+	}
+
+	if (ret)
+		aspeed_smc_unregister(controller);
+
+	return ret;
+}
+
+static int aspeed_smc_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	struct aspeed_smc_controller *controller;
+	const struct of_device_id *match;
+	const struct aspeed_smc_info *info;
+	struct resource *res;
+	int ret;
+
+	match = of_match_device(aspeed_smc_matches, &pdev->dev);
+	if (!match || !match->data)
+		return -ENODEV;
+	info = match->data;
+
+	controller = devm_kzalloc(&pdev->dev, sizeof(*controller) +
+		info->nce * sizeof(controller->chips[0]), GFP_KERNEL);
+	if (!controller)
+		return -ENOMEM;
+	controller->info = info;
+	controller->dev = dev;
+
+	mutex_init(&controller->mutex);
+	platform_set_drvdata(pdev, controller);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	controller->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(controller->regs)) {
+		dev_err(dev, "Cannot remap controller address.\n");
+		return PTR_ERR(controller->regs);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	controller->ahb_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(controller->ahb_base)) {
+		dev_err(dev, "Cannot remap controller address.\n");
+		return PTR_ERR(controller->ahb_base);
+	}
+
+	ret = aspeed_smc_setup_flash(controller, np, res);
+	if (ret)
+		dev_err(dev, "Aspeed SMC probe failed %d\n", ret);
+
+	return ret;
+}
+
+static struct platform_driver aspeed_smc_driver = {
+	.probe = aspeed_smc_probe,
+	.remove = aspeed_smc_remove,
+	.driver = {
+		.name = DEVICE_NAME,
+		.of_match_table = aspeed_smc_matches,
+	}
+};
+
+module_platform_driver(aspeed_smc_driver);
+
+MODULE_DESCRIPTION("ASPEED Static Memory Controller Driver");
+MODULE_AUTHOR("Milton Miller");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related

* [PATCH v3 2/5] mtd: aspeed: add memory controllers for the Aspeed AST2400 SoC
From: Cédric Le Goater @ 2016-12-09 16:49 UTC (permalink / raw)
  To: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Cyrille Pitchen,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
	Joel Stanley, Cédric Le Goater
In-Reply-To: <1481302167-28044-1-git-send-email-clg-Bxea+6Xhats@public.gmane.org>

This driver adds mtd support for the Aspeed AST2400 SoC static memory
controllers:

 * New Static Memory Controller (referred as FMC)
   . BMC firmware
   . AST2500 compatible register set
   . 5 chip select pins (CE0 ∼ CE4)
   . supports NOR flash, NAND flash and SPI flash memory.

 * SPI Flash Controller (SPI)
   . host Firmware
   . slightly different register set, between AST2500 and the legacy
     controller
   . supports SPI flash memory
   . 1 chip select pin (CE0)

The legacy static memory controller (referred as SMC) is not
supported, as well as types other than SPI.

Signed-off-by: Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org>
---
 drivers/mtd/spi-nor/Kconfig      |  2 +-
 drivers/mtd/spi-nor/aspeed-smc.c | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 5c0efbd9dd89..22bea563f9bc 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -35,7 +35,7 @@ config SPI_ASPEED
 	depends on HAS_IOMEM && OF
 	help
 	  This enables support for the Firmware Memory controller (FMC)
-	  in the Aspeed AST2500 SoC when attached to SPI NOR chips,
+	  in the Aspeed AST2500/AST2400 SoCs when attached to SPI NOR chips,
 	  and support for the SPI flash memory controller (SPI) for
 	  the host firmware. The implementation only supports SPI NOR.
 
diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
index 6f9244f07aef..99302b0d7786 100644
--- a/drivers/mtd/spi-nor/aspeed-smc.c
+++ b/drivers/mtd/spi-nor/aspeed-smc.c
@@ -119,8 +119,27 @@ struct aspeed_smc_info {
 	void (*set_4b)(struct aspeed_smc_chip *chip);
 };
 
+static void aspeed_smc_chip_set_4b_spi_2400(struct aspeed_smc_chip *chip);
 static void aspeed_smc_chip_set_4b(struct aspeed_smc_chip *chip);
 
+static const struct aspeed_smc_info fmc_2400_info = {
+	.maxsize = 64 * 1024 * 1024,
+	.nce = 5,
+	.hastype = true,
+	.we0 = 16,
+	.ctl0 = 0x10,
+	.set_4b = aspeed_smc_chip_set_4b,
+};
+
+static const struct aspeed_smc_info spi_2400_info = {
+	.maxsize = 64 * 1024 * 1024,
+	.nce = 1,
+	.hastype = false,
+	.we0 = 0,
+	.ctl0 = 0x04,
+	.set_4b = aspeed_smc_chip_set_4b_spi_2400,
+};
+
 static const struct aspeed_smc_info fmc_2500_info = {
 	.maxsize = 256 * 1024 * 1024,
 	.nce = 3,
@@ -210,6 +229,7 @@ struct aspeed_smc_controller {
 #define CONTROL_IO_DUMMY_HI		BIT(14)
 #define CONTROL_IO_DUMMY_HI_SHIFT	14
 #define CONTROL_CLK_DIV4		BIT(13) /* others */
+#define CONTROL_IO_ADDRESS_4B		BIT(13) /* AST2400 SPI */
 #define CONTROL_RW_MERGE		BIT(12)
 #define CONTROL_IO_DUMMY_LO_SHIFT	6
 #define CONTROL_IO_DUMMY_LO		GENMASK(7, \
@@ -406,6 +426,8 @@ static int aspeed_smc_remove(struct platform_device *dev)
 }
 
 static const struct of_device_id aspeed_smc_matches[] = {
+	{ .compatible = "aspeed,ast2400-fmc", .data = &fmc_2400_info },
+	{ .compatible = "aspeed,ast2400-spi", .data = &spi_2400_info },
 	{ .compatible = "aspeed,ast2500-fmc", .data = &fmc_2500_info },
 	{ .compatible = "aspeed,ast2500-spi", .data = &spi_2500_info },
 	{ }
@@ -479,6 +501,17 @@ static void aspeed_smc_chip_set_4b(struct aspeed_smc_chip *chip)
 	}
 }
 
+/*
+ * The AST2400 SPI flash controller does not have a CE Control
+ * register. It uses the CE0 control register to set 4Byte mode at the
+ * controller level.
+ */
+static void aspeed_smc_chip_set_4b_spi_2400(struct aspeed_smc_chip *chip)
+{
+	chip->ctl_val[smc_base] |= CONTROL_IO_ADDRESS_4B;
+	chip->ctl_val[smc_read] |= CONTROL_IO_ADDRESS_4B;
+}
+
 static int aspeed_smc_chip_setup_init(struct aspeed_smc_chip *chip,
 				      struct resource *res)
 {
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 3/5] mtd: aspeed: used a label property
From: Cédric Le Goater @ 2016-12-09 16:49 UTC (permalink / raw)
  To: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Cyrille Pitchen,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
	Joel Stanley, Cédric Le Goater
In-Reply-To: <1481302167-28044-1-git-send-email-clg-Bxea+6Xhats@public.gmane.org>

This can be used to easily identify a chip on a system with multiple
chips.

Signed-off-by: Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org>
---
 drivers/mtd/spi-nor/aspeed-smc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
index 99302b0d7786..9119c0ca86b6 100644
--- a/drivers/mtd/spi-nor/aspeed-smc.c
+++ b/drivers/mtd/spi-nor/aspeed-smc.c
@@ -676,6 +676,8 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 		nor->prepare = aspeed_smc_prep;
 		nor->unprepare = aspeed_smc_unprep;
 
+		mtd->name = of_get_property(child, "label", NULL);
+
 		ret = aspeed_smc_chip_setup_init(chip, r);
 		if (ret)
 			break;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 4/5] mtd: spi-nor: bindings for the Aspeed memory controllers
From: Cédric Le Goater @ 2016-12-09 16:49 UTC (permalink / raw)
  To: linux-mtd
  Cc: Mark Rutland, Boris Brezillon, devicetree, Richard Weinberger,
	Marek Vasut, Rob Herring, Joel Stanley, Cyrille Pitchen,
	Brian Norris, David Woodhouse, Cédric Le Goater
In-Reply-To: <1481302167-28044-1-git-send-email-clg@kaod.org>

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 .../devicetree/bindings/mtd/aspeed-smc.txt         | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/aspeed-smc.txt

diff --git a/Documentation/devicetree/bindings/mtd/aspeed-smc.txt b/Documentation/devicetree/bindings/mtd/aspeed-smc.txt
new file mode 100644
index 000000000000..e2c88cea38f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/aspeed-smc.txt
@@ -0,0 +1,51 @@
+* Aspeed Firmware Memory controller
+* Aspeed SPI Flash Memory Controller
+
+The Firmware Memory Controller in the Aspeed AST2500 SoC supports
+three chip selects, two of which are always of SPI type and the third
+can be SPI or NOR type flash, but the driver only supports SPI.
+
+The two SPI flash memory controllers in the AST2500 each support two
+chip selects.
+
+Required properties:
+  - compatible : Should be one of
+	"aspeed,ast2400-fmc" for the AST2400 Firmware Memory Controller
+	"aspeed,ast2400-spi" for the AST2400 SPI Flash memory Controller
+	"aspeed,ast2500-fmc" for the AST2500 Firmware Memory Controller
+	"aspeed,ast2500-spi" for the AST2500 SPI flash memory controllers
+
+  - reg : the first contains the control register location and length,
+          the second contains the memory window mapping address and length
+  - #address-cells : must be 1 corresponding to chip select child binding
+  - #size-cells : must be 0 corresponding to chip select child binding
+
+Optional properties:
+  - interrupts : Should contain the interrupt for the dma device if an
+    FMC
+
+The child nodes are the SPI flash modules which must have a compatible
+property as specified in bindings/mtd/jedec,spi-nor.txt
+
+Optionally, the child node can contain properties for SPI mode (may be
+ignored):
+  - spi-max-frequency - max frequency of spi bus
+
+
+Example:
+fmc: fmc@1e620000 {
+	compatible = "aspeed,ast2500-fmc";
+	reg = < 0x1e620000 0x94
+		0x20000000 0x02000000 >;
+	#address-cells = <1>;
+	#size-cells = <0>;
+	interrupts = <19>;
+	flash@0 {
+		reg = < 0 >;
+		compatible = "jedec,spi-nor";
+		/* spi-max-frequency = <>; */
+		/* m25p,fast-read; */
+		#address-cells = <1>;
+		#size-cells = <1>;
+	};
+};
-- 
2.7.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related

* [PATCH v3 5/5] mtd: spi-nor: add a label property to jedec,spi-nor
From: Cédric Le Goater @ 2016-12-09 16:49 UTC (permalink / raw)
  To: linux-mtd
  Cc: Mark Rutland, Boris Brezillon, devicetree, Richard Weinberger,
	Marek Vasut, Rob Herring, Joel Stanley, Cyrille Pitchen,
	Brian Norris, David Woodhouse, Cédric Le Goater
In-Reply-To: <1481302167-28044-1-git-send-email-clg@kaod.org>

This can be used to easily identify a chip on a system with multiple
chips.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
index 2c91c03e7eb0..b7cd02a3ebe8 100644
--- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
+++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
@@ -65,6 +65,7 @@ Optional properties:
                    all chips and support for it can not be detected at runtime.
                    Refer to your chips' datasheet to check if this is supported
                    by your chip.
+- label : name to assign to mtd. If omitted, the label is the MTD device name.
 
 Example:
 
@@ -75,4 +76,5 @@ Example:
 		reg = <0>;
 		spi-max-frequency = <40000000>;
 		m25p,fast-read;
+		label = "System-firmware";
 	};
-- 
2.7.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related

* [PATCH v2 0/7] Add support for Video Data Order Adapter
From: Michael Tretter @ 2016-12-09 16:58 UTC (permalink / raw)
  To: linux-media-u79uwXL29TY76Z2rM5mHXA
  Cc: Philipp Zabel, devicetree-u79uwXL29TY76Z2rM5mHXA, Hans Verkuil,
	Mauro Carvalho Chehab, Michael Tretter

Hello,

This is v2 of a patch series that adds support for the Video Data Order
Adapter (VDOA) that can be found on Freescale i.MX6. It converts the
macroblock tiled format produced by the CODA 960 video decoder to a
raster-ordered format for scanout.

Changes since v1:

- Dropped patch 8/9 of v1
- Patch 1/7: Add devicetree binding documentation for fsl-vdoa
- Patch 6/7: I merged patch 5/9 and patch 8/9 of v1 into a single patch
- Patch 6/7: Use dt compatible instead of a phandle to find VDOA device
- Patch 6/7: Always check VDOA availability even if disabled via module
  parameter and do not print a message if VDOA cannot be found
- Patch 6/7: Do not change the CODA context in coda_try_fmt()
- Patch 6/7: Allocate an additional internal frame if the VDOA is in use

Michael


Michael Tretter (3):
  [media] coda: fix frame index to returned error
  [media] coda: use VDOA for un-tiling custom macroblock format
  [media] coda: support YUYV output if VDOA is used

Philipp Zabel (4):
  ARM: dts: imx6qdl: Add VDOA compatible and clocks properties
  [media] coda: add i.MX6 VDOA driver
  [media] coda: correctly set capture compose rectangle
  [media] coda: add debug output about tiling

 .../devicetree/bindings/media/fsl-vdoa.txt         |  21 ++
 arch/arm/boot/dts/imx6qdl.dtsi                     |   2 +
 drivers/media/platform/Kconfig                     |   3 +
 drivers/media/platform/coda/Makefile               |   1 +
 drivers/media/platform/coda/coda-bit.c             |  93 ++++--
 drivers/media/platform/coda/coda-common.c          | 175 ++++++++++-
 drivers/media/platform/coda/coda.h                 |   3 +
 drivers/media/platform/coda/imx-vdoa.c             | 335 +++++++++++++++++++++
 drivers/media/platform/coda/imx-vdoa.h             |  58 ++++
 9 files changed, 649 insertions(+), 42 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/fsl-vdoa.txt
 create mode 100644 drivers/media/platform/coda/imx-vdoa.c
 create mode 100644 drivers/media/platform/coda/imx-vdoa.h

-- 
2.10.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

* [PATCH v2 1/7] ARM: dts: imx6qdl: Add VDOA compatible and clocks properties
From: Michael Tretter @ 2016-12-09 16:58 UTC (permalink / raw)
  To: linux-media
  Cc: Philipp Zabel, devicetree, Hans Verkuil, Mauro Carvalho Chehab,
	Philipp Zabel, Michael Tretter
In-Reply-To: <20161209165903.1293-1-m.tretter@pengutronix.de>

From: Philipp Zabel <philipp.zabel@gmail.com>

This adds a compatible property and the correct clock for the
i.MX6Q Video Data Order Adapter.

Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 .../devicetree/bindings/media/fsl-vdoa.txt          | 21 +++++++++++++++++++++
 arch/arm/boot/dts/imx6qdl.dtsi                      |  2 ++
 2 files changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/fsl-vdoa.txt

diff --git a/Documentation/devicetree/bindings/media/fsl-vdoa.txt b/Documentation/devicetree/bindings/media/fsl-vdoa.txt
new file mode 100644
index 0000000..5e45f9b
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/fsl-vdoa.txt
@@ -0,0 +1,21 @@
+Freescale Video Data Order Adapter
+==================================
+
+The Video Data Order Adapter (VDOA) is present on the i.MX6q. Its sole purpose
+it to to reorder video data from the macroblock tiled order produced by the
+CODA 960 VPU to the conventional raster-scan order for scanout.
+
+Required properties:
+- compatible: must be "fsl,imx6q-vdoa"
+- reg: the register base and size for the device registers
+- interrupts: the VDOA interrupt
+- clocks: the vdoa clock
+
+Example:
+
+vdoa@021e4000 {
+        compatible = "fsl,imx6q-vdoa";
+        reg = <0x021e4000 0x4000>;
+        interrupts = <0 18 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&clks IMX6QDL_CLK_VDOA>;
+};
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index b13b0b2..69e3668 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -1153,8 +1153,10 @@
 			};
 
 			vdoa@021e4000 {
+				compatible = "fsl,imx6q-vdoa";
 				reg = <0x021e4000 0x4000>;
 				interrupts = <0 18 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clks IMX6QDL_CLK_VDOA>;
 			};
 
 			uart2: serial@021e8000 {
-- 
2.10.2

^ permalink raw reply related

* [PATCH v2 2/7] [media] coda: add i.MX6 VDOA driver
From: Michael Tretter @ 2016-12-09 16:58 UTC (permalink / raw)
  To: linux-media
  Cc: Philipp Zabel, devicetree, Hans Verkuil, Mauro Carvalho Chehab,
	Philipp Zabel, Michael Tretter
In-Reply-To: <20161209165903.1293-1-m.tretter@pengutronix.de>

From: Philipp Zabel <philipp.zabel@gmail.com>

The i.MX6 Video Data Order Adapter's (VDOA) sole purpose is to convert
from a custom macroblock tiled format produced by the CODA960 decoder
into linear formats that can be used for scanout.

Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/media/platform/Kconfig         |   3 +
 drivers/media/platform/coda/Makefile   |   1 +
 drivers/media/platform/coda/imx-vdoa.c | 335 +++++++++++++++++++++++++++++++++
 drivers/media/platform/coda/imx-vdoa.h |  58 ++++++
 4 files changed, 397 insertions(+)
 create mode 100644 drivers/media/platform/coda/imx-vdoa.c
 create mode 100644 drivers/media/platform/coda/imx-vdoa.h

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index ce4a96f..41e007f 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -162,6 +162,9 @@ config VIDEO_CODA
 	   Coda is a range of video codec IPs that supports
 	   H.264, MPEG-4, and other video formats.
 
+config VIDEO_IMX_VDOA
+	def_tristate VIDEO_CODA if SOC_IMX6Q || COMPILE_TEST
+
 config VIDEO_MEDIATEK_VPU
 	tristate "Mediatek Video Processor Unit"
 	depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
diff --git a/drivers/media/platform/coda/Makefile b/drivers/media/platform/coda/Makefile
index 9342ac5..8582843 100644
--- a/drivers/media/platform/coda/Makefile
+++ b/drivers/media/platform/coda/Makefile
@@ -3,3 +3,4 @@ ccflags-y += -I$(src)
 coda-objs := coda-common.o coda-bit.o coda-gdi.o coda-h264.o coda-jpeg.o
 
 obj-$(CONFIG_VIDEO_CODA) += coda.o
+obj-$(CONFIG_VIDEO_IMX_VDOA) += imx-vdoa.o
diff --git a/drivers/media/platform/coda/imx-vdoa.c b/drivers/media/platform/coda/imx-vdoa.c
new file mode 100644
index 0000000..36ceffb
--- /dev/null
+++ b/drivers/media/platform/coda/imx-vdoa.c
@@ -0,0 +1,335 @@
+/*
+ * i.MX6 Video Data Order Adapter (VDOA)
+ *
+ * Copyright (C) 2014 Philipp Zabel
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/dma-mapping.h>
+#include <linux/platform_device.h>
+#include <linux/videodev2.h>
+#include <linux/slab.h>
+
+#include "imx-vdoa.h"
+
+#define VDOA_NAME "imx-vdoa"
+
+#define VDOAC		0x00
+#define VDOASRR		0x04
+#define VDOAIE		0x08
+#define VDOAIST		0x0c
+#define VDOAFP		0x10
+#define VDOAIEBA00	0x14
+#define VDOAIEBA01	0x18
+#define VDOAIEBA02	0x1c
+#define VDOAIEBA10	0x20
+#define VDOAIEBA11	0x24
+#define VDOAIEBA12	0x28
+#define VDOASL		0x2c
+#define VDOAIUBO	0x30
+#define VDOAVEBA0	0x34
+#define VDOAVEBA1	0x38
+#define VDOAVEBA2	0x3c
+#define VDOAVUBO	0x40
+#define VDOASR		0x44
+
+#define VDOAC_ISEL		BIT(6)
+#define VDOAC_PFS		BIT(5)
+#define VDOAC_SO		BIT(4)
+#define VDOAC_SYNC		BIT(3)
+#define VDOAC_NF		BIT(2)
+#define VDOAC_BNDM_MASK		0x3
+#define VDOAC_BAND_HEIGHT_8	0x0
+#define VDOAC_BAND_HEIGHT_16	0x1
+#define VDOAC_BAND_HEIGHT_32	0x2
+
+#define VDOASRR_START		BIT(1)
+#define VDOASRR_SWRST		BIT(0)
+
+#define VDOAIE_EITERR		BIT(1)
+#define VDOAIE_EIEOT		BIT(0)
+
+#define VDOAIST_TERR		BIT(1)
+#define VDOAIST_EOT		BIT(0)
+
+#define VDOAFP_FH_MASK		(0x1fff << 16)
+#define VDOAFP_FW_MASK		(0x3fff)
+
+#define VDOASL_VSLY_MASK	(0x3fff << 16)
+#define VDOASL_ISLY_MASK	(0x7fff)
+
+#define VDOASR_ERRW		BIT(4)
+#define VDOASR_EOB		BIT(3)
+#define VDOASR_CURRENT_FRAME	(0x3 << 1)
+#define VDOASR_CURRENT_BUFFER	BIT(1)
+
+enum {
+	V4L2_M2M_SRC = 0,
+	V4L2_M2M_DST = 1,
+};
+
+struct vdoa_data {
+	struct vdoa_ctx		*curr_ctx;
+	struct device		*dev;
+	struct clk		*vdoa_clk;
+	void __iomem		*regs;
+	int			irq;
+};
+
+struct vdoa_q_data {
+	unsigned int	width;
+	unsigned int	height;
+	unsigned int	bytesperline;
+	unsigned int	sizeimage;
+	u32		pixelformat;
+};
+
+struct vdoa_ctx {
+	struct vdoa_data	*vdoa;
+	struct completion	completion;
+	struct vdoa_q_data	q_data[2];
+};
+
+static irqreturn_t vdoa_irq_handler(int irq, void *data)
+{
+	struct vdoa_data *vdoa = data;
+	struct vdoa_ctx *curr_ctx;
+	u32 val;
+
+	/* Disable interrupts */
+	writel(0, vdoa->regs + VDOAIE);
+
+	curr_ctx = vdoa->curr_ctx;
+	if (!curr_ctx) {
+		dev_dbg(vdoa->dev,
+			"Instance released before the end of transaction\n");
+		return IRQ_HANDLED;
+	}
+
+	val = readl(vdoa->regs + VDOAIST);
+	writel(val, vdoa->regs + VDOAIST);
+	if (val & VDOAIST_TERR) {
+		val = readl(vdoa->regs + VDOASR) & VDOASR_ERRW;
+		dev_err(vdoa->dev, "AXI %s error\n", val ? "write" : "read");
+	} else if (!(val & VDOAIST_EOT)) {
+		dev_warn(vdoa->dev, "Spurious interrupt\n");
+	}
+	complete(&curr_ctx->completion);
+
+	return IRQ_HANDLED;
+}
+
+void vdoa_device_run(struct vdoa_ctx *ctx, dma_addr_t dst, dma_addr_t src)
+{
+	struct vdoa_q_data *src_q_data, *dst_q_data;
+	struct vdoa_data *vdoa = ctx->vdoa;
+	u32 val;
+
+	vdoa->curr_ctx = ctx;
+
+	src_q_data = &ctx->q_data[V4L2_M2M_SRC];
+	dst_q_data = &ctx->q_data[V4L2_M2M_DST];
+
+	/* Progressive, no sync, 1 frame per run */
+	if (dst_q_data->pixelformat == V4L2_PIX_FMT_YUYV)
+		val = VDOAC_PFS;
+	else
+		val = 0;
+	writel(val, vdoa->regs + VDOAC);
+
+	writel(dst_q_data->height << 16 | dst_q_data->width,
+	       vdoa->regs + VDOAFP);
+
+	val = dst;
+	writel(val, vdoa->regs + VDOAIEBA00);
+
+	writel(src_q_data->bytesperline << 16 | dst_q_data->bytesperline,
+	       vdoa->regs + VDOASL);
+
+	if (dst_q_data->pixelformat == V4L2_PIX_FMT_NV12 ||
+	    dst_q_data->pixelformat == V4L2_PIX_FMT_NV21)
+		val = dst_q_data->bytesperline * dst_q_data->height;
+	else
+		val = 0;
+	writel(val, vdoa->regs + VDOAIUBO);
+
+	val = src;
+	writel(val, vdoa->regs + VDOAVEBA0);
+	val = src_q_data->bytesperline * src_q_data->height;
+	writel(val, vdoa->regs + VDOAVUBO);
+
+	/* Enable interrupts and start transfer */
+	writel(VDOAIE_EITERR | VDOAIE_EIEOT, vdoa->regs + VDOAIE);
+	writel(VDOASRR_START, vdoa->regs + VDOASRR);
+}
+EXPORT_SYMBOL(vdoa_device_run);
+
+int vdoa_wait_for_completion(struct vdoa_ctx *ctx)
+{
+	struct vdoa_data *vdoa = ctx->vdoa;
+
+	if (!wait_for_completion_timeout(&ctx->completion,
+					 msecs_to_jiffies(300))) {
+		dev_err(vdoa->dev,
+			"Timeout waiting for transfer result\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(vdoa_wait_for_completion);
+
+struct vdoa_ctx *vdoa_context_create(struct vdoa_data *vdoa)
+{
+	struct vdoa_ctx *ctx;
+	int err;
+
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return NULL;
+
+	err = clk_prepare_enable(vdoa->vdoa_clk);
+	if (err) {
+		kfree(ctx);
+		return NULL;
+	}
+
+	init_completion(&ctx->completion);
+	ctx->vdoa = vdoa;
+
+	return ctx;
+}
+EXPORT_SYMBOL(vdoa_context_create);
+
+void vdoa_context_destroy(struct vdoa_ctx *ctx)
+{
+	struct vdoa_data *vdoa = ctx->vdoa;
+
+	clk_disable_unprepare(vdoa->vdoa_clk);
+	kfree(ctx);
+}
+EXPORT_SYMBOL(vdoa_context_destroy);
+
+int vdoa_context_configure(struct vdoa_ctx *ctx,
+			   unsigned int width, unsigned int height,
+			   u32 pixelformat)
+{
+	struct vdoa_q_data *src_q_data;
+	struct vdoa_q_data *dst_q_data;
+
+	if (width < 16 || width  > 8192 || width % 16 != 0 ||
+	    height < 16 || height > 4096 || height % 16 != 0)
+		return -EINVAL;
+
+	if (pixelformat != V4L2_PIX_FMT_YUYV &&
+	    pixelformat != V4L2_PIX_FMT_NV12)
+		return -EINVAL;
+
+	/* If no context is passed, only check if the format is valid */
+	if (!ctx)
+		return 0;
+
+	src_q_data = &ctx->q_data[V4L2_M2M_SRC];
+	dst_q_data = &ctx->q_data[V4L2_M2M_DST];
+
+	src_q_data->width = width;
+	src_q_data->height = height;
+	src_q_data->bytesperline = width;
+	src_q_data->sizeimage = src_q_data->bytesperline * height * 3 / 2;
+
+	dst_q_data->width = width;
+	dst_q_data->height = height;
+	dst_q_data->pixelformat = pixelformat;
+	switch (pixelformat) {
+	case V4L2_PIX_FMT_YUYV:
+		dst_q_data->bytesperline = width * 2;
+		dst_q_data->sizeimage = dst_q_data->bytesperline * height;
+		break;
+	case V4L2_PIX_FMT_NV12:
+	default:
+		dst_q_data->bytesperline = width;
+		dst_q_data->sizeimage =
+			dst_q_data->bytesperline * height * 3 / 2;
+		break;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(vdoa_context_configure);
+
+static int vdoa_probe(struct platform_device *pdev)
+{
+	struct vdoa_data *vdoa;
+	struct resource *res;
+
+	dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
+
+	vdoa = devm_kzalloc(&pdev->dev, sizeof(*vdoa), GFP_KERNEL);
+	if (!vdoa)
+		return -ENOMEM;
+
+	vdoa->dev = &pdev->dev;
+
+	vdoa->vdoa_clk = devm_clk_get(vdoa->dev, NULL);
+	if (IS_ERR(vdoa->vdoa_clk)) {
+		dev_err(vdoa->dev, "Failed to get clock\n");
+		return PTR_ERR(vdoa->vdoa_clk);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	vdoa->regs = devm_ioremap_resource(vdoa->dev, res);
+	if (IS_ERR(vdoa->regs))
+		return PTR_ERR(vdoa->regs);
+
+	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	vdoa->irq = devm_request_threaded_irq(&pdev->dev, res->start, NULL,
+					vdoa_irq_handler, IRQF_ONESHOT,
+					"vdoa", vdoa);
+	if (vdoa->irq < 0) {
+		dev_err(vdoa->dev, "Failed to get irq\n");
+		return vdoa->irq;
+	}
+
+	platform_set_drvdata(pdev, vdoa);
+
+	return 0;
+}
+
+static int vdoa_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static struct of_device_id vdoa_dt_ids[] = {
+	{ .compatible = "fsl,imx6q-vdoa" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, vdoa_dt_ids);
+
+static struct platform_driver vdoa_driver = {
+	.probe		= vdoa_probe,
+	.remove		= vdoa_remove,
+	.driver		= {
+		.name	= VDOA_NAME,
+		.of_match_table = vdoa_dt_ids,
+	},
+};
+
+module_platform_driver(vdoa_driver);
+
+MODULE_DESCRIPTION("Video Data Order Adapter");
+MODULE_AUTHOR("Philipp Zabel <philipp.zabel@gmail.com>");
+MODULE_ALIAS("platform:imx-vdoa");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/platform/coda/imx-vdoa.h b/drivers/media/platform/coda/imx-vdoa.h
new file mode 100644
index 0000000..967576b
--- /dev/null
+++ b/drivers/media/platform/coda/imx-vdoa.h
@@ -0,0 +1,58 @@
+/*
+ * Copyright (C) 2016 Pengutronix
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef IMX_VDOA_H
+#define IMX_VDOA_H
+
+struct vdoa_data;
+struct vdoa_ctx;
+
+#if (defined CONFIG_VIDEO_IMX_VDOA || defined CONFIG_VIDEO_IMX_VDOA_MODULE)
+
+struct vdoa_ctx *vdoa_context_create(struct vdoa_data *vdoa);
+int vdoa_context_configure(struct vdoa_ctx *ctx,
+			   unsigned int width, unsigned int height,
+			   u32 pixelformat);
+void vdoa_context_destroy(struct vdoa_ctx *ctx);
+
+void vdoa_device_run(struct vdoa_ctx *ctx, dma_addr_t dst, dma_addr_t src);
+int vdoa_wait_for_completion(struct vdoa_ctx *ctx);
+
+#else
+
+static inline struct vdoa_ctx *vdoa_context_create(struct vdoa_data *vdoa)
+{
+	return NULL;
+}
+
+static inline int vdoa_context_configure(struct vdoa_ctx *ctx,
+					 unsigned int width,
+					 unsigned int height,
+					 u32 pixelformat)
+{
+	return 0;
+}
+
+static inline void vdoa_context_destroy(struct vdoa_ctx *ctx) { };
+
+static inline void vdoa_device_run(struct vdoa_ctx *ctx,
+				   dma_addr_t dst, dma_addr_t src) { };
+
+static inline int vdoa_wait_for_completion(struct vdoa_ctx *ctx)
+{
+	return 0;
+};
+
+#endif
+
+#endif /* IMX_VDOA_H */
-- 
2.10.2

^ permalink raw reply related

* [PATCH v2 3/7] [media] coda: correctly set capture compose rectangle
From: Michael Tretter @ 2016-12-09 16:58 UTC (permalink / raw)
  To: linux-media-u79uwXL29TY76Z2rM5mHXA
  Cc: Philipp Zabel, devicetree-u79uwXL29TY76Z2rM5mHXA, Hans Verkuil,
	Mauro Carvalho Chehab, Michael Tretter
In-Reply-To: <20161209165903.1293-1-m.tretter-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

From: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Correctly store the rectangle of valid video data in the destination
q_data before rounding up to macroblock size. This fixes the output
of VIDIOC_G_SELECTION for the capture side compose rectangle.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Signed-off-by: Michael Tretter <m.tretter-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/media/platform/coda/coda-common.c | 37 ++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index c39718a..e0184194 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -566,7 +566,8 @@ static int coda_try_fmt_vid_out(struct file *file, void *priv,
 	return coda_try_fmt(ctx, codec, f);
 }
 
-static int coda_s_fmt(struct coda_ctx *ctx, struct v4l2_format *f)
+static int coda_s_fmt(struct coda_ctx *ctx, struct v4l2_format *f,
+		      struct v4l2_rect *r)
 {
 	struct coda_q_data *q_data;
 	struct vb2_queue *vq;
@@ -589,10 +590,14 @@ static int coda_s_fmt(struct coda_ctx *ctx, struct v4l2_format *f)
 	q_data->height = f->fmt.pix.height;
 	q_data->bytesperline = f->fmt.pix.bytesperline;
 	q_data->sizeimage = f->fmt.pix.sizeimage;
-	q_data->rect.left = 0;
-	q_data->rect.top = 0;
-	q_data->rect.width = f->fmt.pix.width;
-	q_data->rect.height = f->fmt.pix.height;
+	if (r) {
+		q_data->rect = *r;
+	} else {
+		q_data->rect.left = 0;
+		q_data->rect.top = 0;
+		q_data->rect.width = f->fmt.pix.width;
+		q_data->rect.height = f->fmt.pix.height;
+	}
 
 	switch (f->fmt.pix.pixelformat) {
 	case V4L2_PIX_FMT_NV12:
@@ -621,27 +626,37 @@ static int coda_s_fmt_vid_cap(struct file *file, void *priv,
 			      struct v4l2_format *f)
 {
 	struct coda_ctx *ctx = fh_to_ctx(priv);
+	struct coda_q_data *q_data_src;
+	struct v4l2_rect r;
 	int ret;
 
 	ret = coda_try_fmt_vid_cap(file, priv, f);
 	if (ret)
 		return ret;
 
-	return coda_s_fmt(ctx, f);
+	q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
+	r.left = 0;
+	r.top = 0;
+	r.width = q_data_src->width;
+	r.height = q_data_src->height;
+
+	return coda_s_fmt(ctx, f, &r);
 }
 
 static int coda_s_fmt_vid_out(struct file *file, void *priv,
 			      struct v4l2_format *f)
 {
 	struct coda_ctx *ctx = fh_to_ctx(priv);
+	struct coda_q_data *q_data_src;
 	struct v4l2_format f_cap;
+	struct v4l2_rect r;
 	int ret;
 
 	ret = coda_try_fmt_vid_out(file, priv, f);
 	if (ret)
 		return ret;
 
-	ret = coda_s_fmt(ctx, f);
+	ret = coda_s_fmt(ctx, f, NULL);
 	if (ret)
 		return ret;
 
@@ -657,7 +672,13 @@ static int coda_s_fmt_vid_out(struct file *file, void *priv,
 	if (ret)
 		return ret;
 
-	return coda_s_fmt(ctx, &f_cap);
+	q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
+	r.left = 0;
+	r.top = 0;
+	r.width = q_data_src->width;
+	r.height = q_data_src->height;
+
+	return coda_s_fmt(ctx, &f_cap, &r);
 }
 
 static int coda_reqbufs(struct file *file, void *priv,
-- 
2.10.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

* [PATCH v2 4/7] [media] coda: add debug output about tiling
From: Michael Tretter @ 2016-12-09 16:59 UTC (permalink / raw)
  To: linux-media-u79uwXL29TY76Z2rM5mHXA
  Cc: Philipp Zabel, devicetree-u79uwXL29TY76Z2rM5mHXA, Hans Verkuil,
	Mauro Carvalho Chehab, Michael Tretter
In-Reply-To: <20161209165903.1293-1-m.tretter-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

From: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

In order to make the VDOA work correctly, the CODA must produce frames
in tiled format. Print this information in the debug output.

Also print the color format in fourcc instead of the numeric value.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Signed-off-by: Michael Tretter <m.tretter-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/media/platform/coda/coda-common.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index e0184194..f739873 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -616,8 +616,10 @@ static int coda_s_fmt(struct coda_ctx *ctx, struct v4l2_format *f,
 	}
 
 	v4l2_dbg(1, coda_debug, &ctx->dev->v4l2_dev,
-		"Setting format for type %d, wxh: %dx%d, fmt: %d\n",
-		f->type, q_data->width, q_data->height, q_data->fourcc);
+		"Setting format for type %d, wxh: %dx%d, fmt: %4.4s %c\n",
+		f->type, q_data->width, q_data->height,
+		(char *)&q_data->fourcc,
+		(ctx->tiled_map_type == GDI_LINEAR_FRAME_MAP) ? 'L' : 'T');
 
 	return 0;
 }
-- 
2.10.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

* [PATCH v2 5/7] [media] coda: fix frame index to returned error
From: Michael Tretter @ 2016-12-09 16:59 UTC (permalink / raw)
  To: linux-media-u79uwXL29TY76Z2rM5mHXA
  Cc: Philipp Zabel, devicetree-u79uwXL29TY76Z2rM5mHXA, Hans Verkuil,
	Mauro Carvalho Chehab, Michael Tretter
In-Reply-To: <20161209165903.1293-1-m.tretter-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

display_idx refers to the frame that will be returned in the next round.
The currently processed frame is ctx->display_idx and errors should be
reported for this frame.

Signed-off-by: Michael Tretter <m.tretter-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Acked-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/media/platform/coda/coda-bit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c
index b662504..309eb4e 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -2057,7 +2057,7 @@ static void coda_finish_decode(struct coda_ctx *ctx)
 		}
 		vb2_set_plane_payload(&dst_buf->vb2_buf, 0, payload);
 
-		coda_m2m_buf_done(ctx, dst_buf, ctx->frame_errors[display_idx] ?
+		coda_m2m_buf_done(ctx, dst_buf, ctx->frame_errors[ctx->display_idx] ?
 				  VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
 
 		v4l2_dbg(1, coda_debug, &dev->v4l2_dev,
-- 
2.10.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

* [PATCH v2 6/7] [media] coda: use VDOA for un-tiling custom macroblock format
From: Michael Tretter @ 2016-12-09 16:59 UTC (permalink / raw)
  To: linux-media-u79uwXL29TY76Z2rM5mHXA
  Cc: Philipp Zabel, devicetree-u79uwXL29TY76Z2rM5mHXA, Hans Verkuil,
	Mauro Carvalho Chehab, Michael Tretter
In-Reply-To: <20161209165903.1293-1-m.tretter-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

If the CODA driver is configured to produce NV12 output and the VDOA is
available, the VDOA can be used to transform the custom macroblock tiled
format to a raster-ordered format for scanout.

In this case, set the output format of the CODA to the custom macroblock
tiled format, disable the rotator, and use the VDOA to write to the v4l2
buffer. The VDOA is synchronized with the CODA to always un-tile the
frame that the CODA finished in the previous run.

Signed-off-by: Michael Tretter <m.tretter-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/media/platform/coda/coda-bit.c    |  86 +++++++++++++++++--------
 drivers/media/platform/coda/coda-common.c | 102 ++++++++++++++++++++++++++++--
 drivers/media/platform/coda/coda.h        |   3 +
 3 files changed, 161 insertions(+), 30 deletions(-)

diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c
index 309eb4e..f608de4 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -30,6 +30,7 @@
 #include <media/videobuf2-vmalloc.h>
 
 #include "coda.h"
+#include "imx-vdoa.h"
 #define CREATE_TRACE_POINTS
 #include "trace.h"
 
@@ -1517,6 +1518,10 @@ static int __coda_start_decoding(struct coda_ctx *ctx)
 	u32 val;
 	int ret;
 
+	v4l2_dbg(1, coda_debug, &dev->v4l2_dev,
+		 "Video Data Order Adapter: %s\n",
+		 ctx->use_vdoa ? "Enabled" : "Disabled");
+
 	/* Start decoding */
 	q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
 	q_data_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
@@ -1535,7 +1540,8 @@ static int __coda_start_decoding(struct coda_ctx *ctx)
 	if (dst_fourcc == V4L2_PIX_FMT_NV12)
 		ctx->frame_mem_ctrl |= CODA_FRAME_CHROMA_INTERLEAVE;
 	if (ctx->tiled_map_type == GDI_TILED_FRAME_MB_RASTER_MAP)
-		ctx->frame_mem_ctrl |= (0x3 << 9) | CODA9_FRAME_TILED2LINEAR;
+		ctx->frame_mem_ctrl |= (0x3 << 9) |
+			((ctx->use_vdoa) ? 0 : CODA9_FRAME_TILED2LINEAR);
 	coda_write(dev, ctx->frame_mem_ctrl, CODA_REG_BIT_FRAME_MEM_CTRL);
 
 	ctx->display_idx = -1;
@@ -1618,6 +1624,15 @@ static int __coda_start_decoding(struct coda_ctx *ctx)
 		 __func__, ctx->idx, width, height);
 
 	ctx->num_internal_frames = coda_read(dev, CODA_RET_DEC_SEQ_FRAME_NEED);
+	/*
+	 * If the VDOA is used, the decoder needs one additional frame,
+	 * because the frames are freed when the next frame is decoded.
+	 * Otherwise there are visible errors in the decoded frames (green
+	 * regions in displayed frames) and a broken order of frames (earlier
+	 * frames are sporadically displayed after later frames).
+	 */
+	if (ctx->use_vdoa)
+		ctx->num_internal_frames += 1;
 	if (ctx->num_internal_frames > CODA_MAX_FRAMEBUFFERS) {
 		v4l2_err(&dev->v4l2_dev,
 			 "not enough framebuffers to decode (%d < %d)\n",
@@ -1724,6 +1739,7 @@ static int coda_prepare_decode(struct coda_ctx *ctx)
 	struct coda_q_data *q_data_dst;
 	struct coda_buffer_meta *meta;
 	unsigned long flags;
+	u32 rot_mode = 0;
 	u32 reg_addr, reg_stride;
 
 	dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
@@ -1759,27 +1775,40 @@ static int coda_prepare_decode(struct coda_ctx *ctx)
 	if (dev->devtype->product == CODA_960)
 		coda_set_gdi_regs(ctx);
 
-	if (dev->devtype->product == CODA_960) {
-		/*
-		 * The CODA960 seems to have an internal list of buffers with
-		 * 64 entries that includes the registered frame buffers as
-		 * well as the rotator buffer output.
-		 * ROT_INDEX needs to be < 0x40, but > ctx->num_internal_frames.
-		 */
-		coda_write(dev, CODA_MAX_FRAMEBUFFERS + dst_buf->vb2_buf.index,
-				CODA9_CMD_DEC_PIC_ROT_INDEX);
-
-		reg_addr = CODA9_CMD_DEC_PIC_ROT_ADDR_Y;
-		reg_stride = CODA9_CMD_DEC_PIC_ROT_STRIDE;
+	if (ctx->use_vdoa &&
+	    ctx->display_idx >= 0 &&
+	    ctx->display_idx < ctx->num_internal_frames) {
+		vdoa_device_run(ctx->vdoa,
+				vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0),
+				ctx->internal_frames[ctx->display_idx].paddr);
 	} else {
-		reg_addr = CODA_CMD_DEC_PIC_ROT_ADDR_Y;
-		reg_stride = CODA_CMD_DEC_PIC_ROT_STRIDE;
+		if (dev->devtype->product == CODA_960) {
+			/*
+			 * The CODA960 seems to have an internal list of
+			 * buffers with 64 entries that includes the
+			 * registered frame buffers as well as the rotator
+			 * buffer output.
+			 *
+			 * ROT_INDEX needs to be < 0x40, but >
+			 * ctx->num_internal_frames.
+			 */
+			coda_write(dev,
+				   CODA_MAX_FRAMEBUFFERS + dst_buf->vb2_buf.index,
+				   CODA9_CMD_DEC_PIC_ROT_INDEX);
+
+			reg_addr = CODA9_CMD_DEC_PIC_ROT_ADDR_Y;
+			reg_stride = CODA9_CMD_DEC_PIC_ROT_STRIDE;
+		} else {
+			reg_addr = CODA_CMD_DEC_PIC_ROT_ADDR_Y;
+			reg_stride = CODA_CMD_DEC_PIC_ROT_STRIDE;
+		}
+		coda_write_base(ctx, q_data_dst, dst_buf, reg_addr);
+		coda_write(dev, q_data_dst->bytesperline, reg_stride);
+
+		rot_mode = CODA_ROT_MIR_ENABLE | ctx->params.rot_mode;
 	}
-	coda_write_base(ctx, q_data_dst, dst_buf, reg_addr);
-	coda_write(dev, q_data_dst->bytesperline, reg_stride);
 
-	coda_write(dev, CODA_ROT_MIR_ENABLE | ctx->params.rot_mode,
-			CODA_CMD_DEC_PIC_ROT_MODE);
+	coda_write(dev, rot_mode, CODA_CMD_DEC_PIC_ROT_MODE);
 
 	switch (dev->devtype->product) {
 	case CODA_DX6:
@@ -1851,6 +1880,7 @@ static void coda_finish_decode(struct coda_ctx *ctx)
 	u32 src_fourcc;
 	int success;
 	u32 err_mb;
+	int err_vdoa = 0;
 	u32 val;
 
 	/* Update kfifo out pointer from coda bitstream read pointer */
@@ -1934,13 +1964,17 @@ static void coda_finish_decode(struct coda_ctx *ctx)
 		}
 	}
 
+	/* Wait until the VDOA finished writing the previous display frame */
+	if (ctx->use_vdoa &&
+	    ctx->display_idx >= 0 &&
+	    ctx->display_idx < ctx->num_internal_frames) {
+		err_vdoa = vdoa_wait_for_completion(ctx->vdoa);
+	}
+
 	ctx->frm_dis_flg = coda_read(dev,
 				     CODA_REG_BIT_FRM_DIS_FLG(ctx->reg_idx));
 
-	/*
-	 * The previous display frame was copied out by the rotator,
-	 * now it can be overwritten again
-	 */
+	/* The previous display frame was copied out and can be overwritten */
 	if (ctx->display_idx >= 0 &&
 	    ctx->display_idx < ctx->num_internal_frames) {
 		ctx->frm_dis_flg &= ~(1 << ctx->display_idx);
@@ -2057,8 +2091,10 @@ static void coda_finish_decode(struct coda_ctx *ctx)
 		}
 		vb2_set_plane_payload(&dst_buf->vb2_buf, 0, payload);
 
-		coda_m2m_buf_done(ctx, dst_buf, ctx->frame_errors[ctx->display_idx] ?
-				  VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
+		if (ctx->frame_errors[ctx->display_idx] || err_vdoa)
+			coda_m2m_buf_done(ctx, dst_buf, VB2_BUF_STATE_ERROR);
+		else
+			coda_m2m_buf_done(ctx, dst_buf, VB2_BUF_STATE_DONE);
 
 		v4l2_dbg(1, coda_debug, &dev->v4l2_dev,
 			"job finished: decoding frame (%d) (%s)\n",
diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index f739873..c09cafd 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -41,6 +41,7 @@
 #include <media/videobuf2-vmalloc.h>
 
 #include "coda.h"
+#include "imx-vdoa.h"
 
 #define CODA_NAME		"coda"
 
@@ -66,6 +67,10 @@ static int disable_tiling;
 module_param(disable_tiling, int, 0644);
 MODULE_PARM_DESC(disable_tiling, "Disable tiled frame buffers");
 
+static int disable_vdoa;
+module_param(disable_vdoa, int, 0644);
+MODULE_PARM_DESC(disable_vdoa, "Disable Video Data Order Adapter tiled to raster-scan conversion");
+
 void coda_write(struct coda_dev *dev, u32 data, u32 reg)
 {
 	v4l2_dbg(2, coda_debug, &dev->v4l2_dev,
@@ -325,6 +330,31 @@ const char *coda_product_name(int product)
 	}
 }
 
+static struct vdoa_data *coda_get_vdoa_data(void)
+{
+	struct device_node *vdoa_node;
+	struct platform_device *vdoa_pdev;
+	struct vdoa_data *vdoa_data = NULL;
+
+	vdoa_node = of_find_compatible_node(NULL, NULL, "fsl,imx6q-vdoa");
+	if (!vdoa_node)
+		return NULL;
+
+	vdoa_pdev = of_find_device_by_node(vdoa_node);
+	if (!vdoa_pdev)
+		goto out;
+
+	vdoa_data = platform_get_drvdata(vdoa_pdev);
+	if (!vdoa_data)
+		vdoa_data = ERR_PTR(-EPROBE_DEFER);
+
+out:
+	if (vdoa_node)
+		of_node_put(vdoa_node);
+
+	return vdoa_data;
+}
+
 /*
  * V4L2 ioctl() operations.
  */
@@ -417,6 +447,33 @@ static int coda_try_pixelformat(struct coda_ctx *ctx, struct v4l2_format *f)
 	return 0;
 }
 
+static int coda_try_fmt_vdoa(struct coda_ctx *ctx, struct v4l2_format *f,
+			     bool *use_vdoa)
+{
+	int err;
+
+	if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	if (!use_vdoa)
+		return -EINVAL;
+
+	if (!ctx->vdoa) {
+		*use_vdoa = false;
+		return 0;
+	}
+
+	err = vdoa_context_configure(NULL, f->fmt.pix.width, f->fmt.pix.height,
+				     f->fmt.pix.pixelformat);
+	if (err) {
+		*use_vdoa = false;
+		return 0;
+	}
+
+	*use_vdoa = true;
+	return 0;
+}
+
 static unsigned int coda_estimate_sizeimage(struct coda_ctx *ctx, u32 sizeimage,
 					    u32 width, u32 height)
 {
@@ -495,6 +552,7 @@ static int coda_try_fmt_vid_cap(struct file *file, void *priv,
 	const struct coda_codec *codec;
 	struct vb2_queue *src_vq;
 	int ret;
+	bool use_vdoa;
 
 	ret = coda_try_pixelformat(ctx, f);
 	if (ret < 0)
@@ -531,6 +589,10 @@ static int coda_try_fmt_vid_cap(struct file *file, void *priv,
 		f->fmt.pix.bytesperline = round_up(f->fmt.pix.width, 16);
 		f->fmt.pix.sizeimage = f->fmt.pix.bytesperline *
 				       f->fmt.pix.height * 3 / 2;
+
+		ret = coda_try_fmt_vdoa(ctx, f, &use_vdoa);
+		if (ret < 0)
+			return ret;
 	}
 
 	return 0;
@@ -601,11 +663,9 @@ static int coda_s_fmt(struct coda_ctx *ctx, struct v4l2_format *f,
 
 	switch (f->fmt.pix.pixelformat) {
 	case V4L2_PIX_FMT_NV12:
-		if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
-			ctx->tiled_map_type = GDI_TILED_FRAME_MB_RASTER_MAP;
-			if (!disable_tiling)
-				break;
-		}
+		ctx->tiled_map_type = GDI_TILED_FRAME_MB_RASTER_MAP;
+		if (!disable_tiling)
+			break;
 		/* else fall through */
 	case V4L2_PIX_FMT_YUV420:
 	case V4L2_PIX_FMT_YVU420:
@@ -615,6 +675,13 @@ static int coda_s_fmt(struct coda_ctx *ctx, struct v4l2_format *f,
 		break;
 	}
 
+	if (ctx->tiled_map_type == GDI_TILED_FRAME_MB_RASTER_MAP &&
+	    !coda_try_fmt_vdoa(ctx, f, &ctx->use_vdoa) &&
+	    ctx->use_vdoa)
+		vdoa_context_configure(ctx->vdoa, f->fmt.pix.width,
+				       f->fmt.pix.height,
+				       f->fmt.pix.pixelformat);
+
 	v4l2_dbg(1, coda_debug, &ctx->dev->v4l2_dev,
 		"Setting format for type %d, wxh: %dx%d, fmt: %4.4s %c\n",
 		f->type, q_data->width, q_data->height,
@@ -1041,6 +1108,16 @@ static int coda_job_ready(void *m2m_priv)
 		bool stream_end = ctx->bit_stream_param &
 				  CODA_BIT_STREAM_END_FLAG;
 		int num_metas = ctx->num_metas;
+		unsigned int count;
+
+		count = hweight32(ctx->frm_dis_flg);
+		if (ctx->use_vdoa && count >= (ctx->num_internal_frames - 1)) {
+			v4l2_dbg(1, coda_debug, &ctx->dev->v4l2_dev,
+				 "%d: not ready: all internal buffers in use: %d/%d (0x%x)",
+				 ctx->idx, count, ctx->num_internal_frames,
+				 ctx->frm_dis_flg);
+			return 0;
+		}
 
 		if (ctx->hold && !src_bufs) {
 			v4l2_dbg(1, coda_debug, &ctx->dev->v4l2_dev,
@@ -1731,6 +1808,13 @@ static int coda_open(struct file *file)
 	default:
 		ctx->reg_idx = idx;
 	}
+	if (ctx->dev->vdoa && !disable_vdoa) {
+		ctx->vdoa = vdoa_context_create(dev->vdoa);
+		if (!ctx->vdoa)
+			v4l2_warn(&dev->v4l2_dev,
+				  "Failed to create vdoa context: not using vdoa");
+	}
+	ctx->use_vdoa = false;
 
 	/* Power up and upload firmware if necessary */
 	ret = pm_runtime_get_sync(&dev->plat_dev->dev);
@@ -1806,6 +1890,9 @@ static int coda_release(struct file *file)
 	v4l2_dbg(1, coda_debug, &dev->v4l2_dev, "Releasing instance %p\n",
 		 ctx);
 
+	if (ctx->vdoa)
+		vdoa_context_destroy(ctx->vdoa);
+
 	if (ctx->inst_type == CODA_INST_DECODER && ctx->use_bit)
 		coda_bit_stream_end_flag(ctx);
 
@@ -2258,6 +2345,11 @@ static int coda_probe(struct platform_device *pdev)
 	}
 	dev->iram_pool = pool;
 
+	/* Get vdoa_data if supported by the platform */
+	dev->vdoa = coda_get_vdoa_data();
+	if (PTR_ERR(dev->vdoa) == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
 	ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
 	if (ret)
 		return ret;
diff --git a/drivers/media/platform/coda/coda.h b/drivers/media/platform/coda/coda.h
index 53f9666..7ed79eb 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -75,6 +75,7 @@ struct coda_dev {
 	struct platform_device	*plat_dev;
 	const struct coda_devtype *devtype;
 	int			firmware;
+	struct vdoa_data	*vdoa;
 
 	void __iomem		*regs_base;
 	struct clk		*clk_per;
@@ -236,6 +237,8 @@ struct coda_ctx {
 	int				display_idx;
 	struct dentry			*debugfs_entry;
 	bool				use_bit;
+	bool				use_vdoa;
+	struct vdoa_ctx			*vdoa;
 };
 
 extern int coda_debug;
-- 
2.10.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

* [PATCH v2 7/7] [media] coda: support YUYV output if VDOA is used
From: Michael Tretter @ 2016-12-09 16:59 UTC (permalink / raw)
  To: linux-media
  Cc: Philipp Zabel, devicetree, Hans Verkuil, Mauro Carvalho Chehab,
	Michael Tretter
In-Reply-To: <20161209165903.1293-1-m.tretter@pengutronix.de>

The VDOA is able to transform the NV12 custom macroblock tiled format of
the CODA to YUYV format. If and only if the VDOA is available, the
driver can also provide YUYV support.

While the driver is configured to produce YUYV output, the CODA must be
configured to produce NV12 macroblock tiled frames and the VDOA must
transform the intermediate result into the final YUYV output.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda/coda-bit.c    |  7 +++++--
 drivers/media/platform/coda/coda-common.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c
index f608de4..466a44e 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -759,7 +759,7 @@ static void coda9_set_frame_cache(struct coda_ctx *ctx, u32 fourcc)
 		cache_config = 1 << CODA9_CACHE_PAGEMERGE_OFFSET;
 	}
 	coda_write(ctx->dev, cache_size, CODA9_CMD_SET_FRAME_CACHE_SIZE);
-	if (fourcc == V4L2_PIX_FMT_NV12) {
+	if (fourcc == V4L2_PIX_FMT_NV12 || fourcc == V4L2_PIX_FMT_YUYV) {
 		cache_config |= 32 << CODA9_CACHE_LUMA_BUFFER_SIZE_OFFSET |
 				16 << CODA9_CACHE_CR_BUFFER_SIZE_OFFSET |
 				0 << CODA9_CACHE_CB_BUFFER_SIZE_OFFSET;
@@ -1537,7 +1537,7 @@ static int __coda_start_decoding(struct coda_ctx *ctx)
 
 	ctx->frame_mem_ctrl &= ~(CODA_FRAME_CHROMA_INTERLEAVE | (0x3 << 9) |
 				 CODA9_FRAME_TILED2LINEAR);
-	if (dst_fourcc == V4L2_PIX_FMT_NV12)
+	if (dst_fourcc == V4L2_PIX_FMT_NV12 || dst_fourcc == V4L2_PIX_FMT_YUYV)
 		ctx->frame_mem_ctrl |= CODA_FRAME_CHROMA_INTERLEAVE;
 	if (ctx->tiled_map_type == GDI_TILED_FRAME_MB_RASTER_MAP)
 		ctx->frame_mem_ctrl |= (0x3 << 9) |
@@ -2079,6 +2079,9 @@ static void coda_finish_decode(struct coda_ctx *ctx)
 		trace_coda_dec_rot_done(ctx, dst_buf, meta);
 
 		switch (q_data_dst->fourcc) {
+		case V4L2_PIX_FMT_YUYV:
+			payload = width * height * 2;
+			break;
 		case V4L2_PIX_FMT_YUV420:
 		case V4L2_PIX_FMT_YVU420:
 		case V4L2_PIX_FMT_NV12:
diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index c09cafd..43af428 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -95,6 +95,8 @@ void coda_write_base(struct coda_ctx *ctx, struct coda_q_data *q_data,
 	u32 base_cb, base_cr;
 
 	switch (q_data->fourcc) {
+	case V4L2_PIX_FMT_YUYV:
+		/* Fallthrough: IN -H264-> CODA -NV12 MB-> VDOA -YUYV-> OUT */
 	case V4L2_PIX_FMT_NV12:
 	case V4L2_PIX_FMT_YUV420:
 	default:
@@ -201,6 +203,11 @@ static const struct coda_video_device coda_bit_decoder = {
 		V4L2_PIX_FMT_NV12,
 		V4L2_PIX_FMT_YUV420,
 		V4L2_PIX_FMT_YVU420,
+		/*
+		 * If V4L2_PIX_FMT_YUYV should be default,
+		 * set_default_params() must be adjusted.
+		 */
+		V4L2_PIX_FMT_YUYV,
 	},
 };
 
@@ -246,6 +253,7 @@ static u32 coda_format_normalize_yuv(u32 fourcc)
 	case V4L2_PIX_FMT_YUV420:
 	case V4L2_PIX_FMT_YVU420:
 	case V4L2_PIX_FMT_YUV422P:
+	case V4L2_PIX_FMT_YUYV:
 		return V4L2_PIX_FMT_YUV420;
 	default:
 		return fourcc;
@@ -434,6 +442,11 @@ static int coda_try_pixelformat(struct coda_ctx *ctx, struct v4l2_format *f)
 		return -EINVAL;
 
 	for (i = 0; i < CODA_MAX_FORMATS; i++) {
+		/* Skip YUYV if the vdoa is not available */
+		if (!ctx->vdoa && f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE &&
+		    formats[i] == V4L2_PIX_FMT_YUYV)
+			continue;
+
 		if (formats[i] == f->fmt.pix.pixelformat) {
 			f->fmt.pix.pixelformat = formats[i];
 			return 0;
@@ -520,6 +533,11 @@ static int coda_try_fmt(struct coda_ctx *ctx, const struct coda_codec *codec,
 		f->fmt.pix.sizeimage = f->fmt.pix.bytesperline *
 					f->fmt.pix.height * 3 / 2;
 		break;
+	case V4L2_PIX_FMT_YUYV:
+		f->fmt.pix.bytesperline = round_up(f->fmt.pix.width, 16) * 2;
+		f->fmt.pix.sizeimage = f->fmt.pix.bytesperline *
+					f->fmt.pix.height;
+		break;
 	case V4L2_PIX_FMT_YUV422P:
 		f->fmt.pix.bytesperline = round_up(f->fmt.pix.width, 16);
 		f->fmt.pix.sizeimage = f->fmt.pix.bytesperline *
@@ -593,6 +611,15 @@ static int coda_try_fmt_vid_cap(struct file *file, void *priv,
 		ret = coda_try_fmt_vdoa(ctx, f, &use_vdoa);
 		if (ret < 0)
 			return ret;
+
+		if (f->fmt.pix.pixelformat == V4L2_PIX_FMT_YUYV) {
+			if (!use_vdoa)
+				return -EINVAL;
+
+			f->fmt.pix.bytesperline = round_up(f->fmt.pix.width, 16) * 2;
+			f->fmt.pix.sizeimage = f->fmt.pix.bytesperline *
+				f->fmt.pix.height;
+		}
 	}
 
 	return 0;
@@ -662,6 +689,9 @@ static int coda_s_fmt(struct coda_ctx *ctx, struct v4l2_format *f,
 	}
 
 	switch (f->fmt.pix.pixelformat) {
+	case V4L2_PIX_FMT_YUYV:
+		ctx->tiled_map_type = GDI_TILED_FRAME_MB_RASTER_MAP;
+		break;
 	case V4L2_PIX_FMT_NV12:
 		ctx->tiled_map_type = GDI_TILED_FRAME_MB_RASTER_MAP;
 		if (!disable_tiling)
-- 
2.10.2

^ permalink raw reply related

* Re: [PATCH v2 1/2] devicetree: i2c-hid: Add Wacom digitizer + regulator support
From: Rob Herring @ 2016-12-09 17:44 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Benjamin Tissoires, Brian Norris, Jiri Kosina, Caesar Wang,
	open list:ARM/Rockchip SoC...,
	linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Dmitry Torokhov, Mark Rutland
In-Reply-To: <CAD=FV=VBvn-QDBMehCXuuTH7ym8-nTV=VxPs=JRjRBzNdezz+Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Dec 9, 2016 at 10:05 AM, Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> Hi,
>
> On Thu, Dec 8, 2016 at 8:01 AM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>> Just my $0.02.  Feel free to ignore.
>>>
>>> One thought is that I would say that the need to power on the device
>>> explicitly seems more like a board level difference and less like a
>>> difference associated with a particular digitizer.  Said another way,
>>> it seems likely there will be boards with a w9013 without explicit
>>> control of the regulator in software and it seems like there will be
>>> boards with other digitizers where suddenly a new board will come out
>>> that needs explicit control of the regulator.
>>
>> Then either the regulator is optional or you don't say it is a w9013
>> for that board. But if you do need to initialize the device and
>> therefore know what type of device it is, then you need a compatible
>> for the device. It's when things really vary by board that we add DT
>> properties.
>>
>>> In this particular case I feel like we can draw a lot of parallels to
>>> an SDIO bus.
>>>
>>> When you specify an SDIO bus you don't specify what kind of card will
>>> be present, you just say "I've got an SDIO bus" and then the specific
>>> device underneath is probed.  Here we've say "I've got an i2c
>>> connection intended for HID" and then you probe for the HID device
>>> that's on the connection.
>>
>> No, the soldered down devices require all sorts of extra non-SDIO
>> connections and we do specify the device in those cases.
>
> We have never specified the device on boards I've worked with.
>
> On rk3288-veyron, for instance, we might have stuffed a Broadcom 4354
> WiFi chip or a Marvell 8897 WiFi chip.  Some veyron boards have one
> chip and some have the other.  ...and during bringup we even had some
> of the exact same boards where half were stuffed with one chip and
> half with the other.
>
> Nothing in the device tree says which chip is stuffed.  In both cases
> the board uses the same power on sequence for the WiFi chip.  Once
> that is done, we dynamically probe which actual WiFi part is stuffed.

That's good and I'm happy when that works, but it doesn't work in the
general case. I'm not saying you can't do exactly the same thing here.
All I'm asking for is add the properties to the binding AND a
compatible. The kernel can ignore the added compatible. The key point
is if you have additional properties outside of what it means to be
HID-over-I2C, then you must have a compatible for that device. Whether
you enforce that in the driver, I don't give a shit. I do care how it
is documented though and will care when or if we start validating
bindings (I don't think that's the kernel's job).

This is not just a problem with probe-able buses. This dual sourcing
happens with non-probe-able devices too. Look at Hans' series for
Allwinner tablet touchscreens.

> Certainly not all users that have these WiFi chips use the same power
> on sequence.  I have certainly seen development boards for these chips
> where you just insert them into a regular SD card slot.  This is a
> more expensive solution because you need more logic on the board, but
> it shows that the power on sequence is not associated with these
> chips.

If SD slots were the primary target for SDIO cards, we'd be in much
better shape without all the misc side band signals. Many devices I
don't think could be made to work as a card.

>>> Also for an SDIO bus, you've possibly got a regulators / GPIOs /
>>> resets that need to be controlled, but the specific details of these
>>> regulator / GPIOs / resets are specific to a given board and not
>>> necessarily a given SDIO device.
>>
>> It's both. The device defines what is needed and the specs to control
>> them (active states of GPIOs, de/assertion times of resets, supply
>> voltages, etc.). The board only determines what the connections are
>> and if you can control them.
>
> It's not always that simple.  The device says that it needs power and
> resets to happen.  How power is provided and how resets happen is
> awfully board specific.  As per above it is possible that the board
> wouldn't need to be involved above is you want to spend more money /
> power.

We have ways to deal with board specifics. If you want something
completely generic to handle any possible power sequence of any board
and any device, then propose something that does that. That's not what
we have here with 2 properties.

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

* 2??????????;
From: ??? @ 2016-12-09 17:46 UTC (permalink / raw)
  To: devicetest.apk; +Cc: devicetree-u79uwXL29TY76Z2rM5mHXA

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="l8", Size: 72 bytes --]

2¿Í»§ËµµÃÓë×öµÄ²»Ò»ÖÂ;Ëï³½³ïdevicetest.apk

2¿Í»§ËµµÃÓë×öµÄ²»Ò»ÖÂ;
33

[-- Attachment #2: ?c[3]n.xls --]
[-- Type: application/x-msexcel, Size: 28672 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox