Devicetree
 help / color / mirror / Atom feed
* Re: [RESEND PATCH v2 5/7] drm/vc4: Document VEC DT binding
From: Rob Herring @ 2016-12-09 21:00 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: David Airlie, Daniel Vetter,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Florian Fainelli,
	Ray Jui, Scott Branden,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Stephen Warren,
	Lee Jones, Eric Anholt,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1480686493-4813-6-git-send-email-boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

On Fri, Dec 02, 2016 at 02:48:11PM +0100, Boris Brezillon wrote:
> Document the DT binding for the VEC (Video EnCoder) IP.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCHv4 11/15] clk: ti: clockdomain: add clock provider support to clockdomains
From: Tony Lindgren @ 2016-12-09 20:40 UTC (permalink / raw)
  To: Michael Turquette
  Cc: Tero Kristo, Stephen Boyd, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring
In-Reply-To: <148131376868.16621.16082839810419290638@resonance>

* Michael Turquette <mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> [161209 12:02]:
> Quoting Tony Lindgren (2016-12-05 07:25:34)
> > * Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org> [161205 02:09]:
...
<snip>

> I had a recent conversation with Kevin Hilman about a related issue
> (we were not discussing this thread or this series) and we both agreed
> that most drivers don't even need to managed their clocks directly, so
> much as they need to manage their on/off resources. Clocks are just one
> part of that, and if we can hide that stuff inside of an attached genpd
> then it would be better than having the driver manage clocks explicitly.
> 
> Obviously some devices such as audio codec or uart will need to manage
> clocks directly, but this is mostly the exception, not the rule.

Yes. And we do that already for clkctrl clocks via PM runtime where
hwmod manages them. Tero's series still has hwmod manage the clocks,
but the clock register handling is moved to live under drivers/clock.

> > > > > There is certainly no API for that in the clock framework, but for genpd
> > > > > your runtime_pm_get() callback for clkdm_A could call runtime_pm_get
> > > > > against clkdm_B, which would satisfy the requirement. See section
> > > > > 3.1.1.1.7 Clock Domain Dependency in the OMAP4 TRM, version AB.
> > > 
> > > For static dependencies the apis genpd_add/remove_subdomain could probably
> > > be used.
> > > 
> > > > To me it seems the API is just clk_get() :) Do you have some
> > > > specific example we can use to check? My guess is that the
> > > > TRM "Clock Domain Dependency" is just the usual parent child
> > > > relationship between clocks that are the clockdomains..
> 
> clk_get() only fetches a pointer to the clk. I guess you mean
> clk_prepare_enable() to actually increment the use count?

Right, with clocks that's all we should need to do :)

> If we used the clk framework here is that it would look something like
> this:
> 
> clk_enable(clk_a)
> -> .enable(clk_a_hw)
>    -> clk_enable(clk_b)
> 
> However, clk_a and clk_b do not have a parent-child relationship in the
> clock tree. This is purely a functional relationship between IP blocks.
> Modeling this sort of thing in the clk framework would be wrong, and
> genpd is a much better place to establish these arbitrary relationships.

Hmm yes, and I don't mean the clock framework should do anything more
complex beyond what it already does.

We just want to represent the clocks as clocks, then have the
interconnect code manage those clocks. That's currently hwmod, eventually
it will be genpd.

> > > > If there is something more magical there certainly that should
> > > > be considered though.
> > > 
> > > The hwmods could be transformed to individual genpds also I guess. On DT
> > > level though, we would still need a clock pointer to the main clock and a
> > > genpd pointer in addition to that.
> > 
> > Hmm a genpd pointer to where exactly? AFAIK each interconnect
> > instance should be a genpd provider, and the individual interconnect
> > target modules should be consumers for that genpd.
> 
> I was thinking that the clock domains would be modeled as genpd objects
> with the interconnect target modules attached as struct devices.

I think clock domains should be just clocks, then we let the interconnect
code and eventually genpd manage them.

> > > Tony, any thoughts on that? Would this break up the plans for the
> > > interconnect completely?
> > 
> > Does using genpd for clockdomains cause issues for using genpd for
> > interconnect instances and the target modules?
> 
> Can they be the same object in Linux? If there is a one-to-one mapping
> between clock domains and the interconnect port then maybe you can just
> model them together.

I'm thinking that it should be the interconnect code implementing
genpd, and use clk_request_enable().

> > The thing I'd be worried about there is that the clockdomains and
> > their child clocks are just devices sitting on the interconnect,
> > so we could easily end up with genpd modeling something that does
> > not represent the hardware.
> > 
> > For example, on 4430 we have:
> > 
> > l4_cfg interconnect
> >        ...
> >        segment@0
> >                 ...
> >                 target_module@4000
> >                         cm1: cm1@0
> 
> How about:
> 
> l4_cfg interconnect
>        ...
>        segment@0
>                 ...
>                 cm1@4000
>                 	module: foo_module@0

That's the wrong way around from hardware point of view. There's
a generic interconnect wrapper module with it's own registers,
then cm1 (and possibly other devices) are children of that target
module.

> I don't know much about the segments. Do they map one-to-one with the
> clock domains?

I need to check, it's been a while, but I recall some interconnects
are partioned to segments based on voltages or clocks.

> If my quick-and-dirty DT above makes sense, then the target modules
> (e.g. io controller) would not get clocks anymore, but just
> pm_runtime_get(). The genpd backing object would call clk_enable/disable
> as needed.

Yeah that's what we already have with hwmod and PM runtime for the
clockctrl register. But hwmod currently directly manages the clkctrl
register, we just want to move that part to be a clock driver.

The children of the interconnect target modules just need to use
PM runtime, but the interconnect target module driver needs to know
it's clkctrl clock.

> If fine grained control of a clock is needed (e.g. for clk_set_rate)
> then the driver can still clk_get it. Whether or not the clockdomain
> provides that clock or if it comes from the clock generator (e.g. cm1,
> cm2, prm, etc) isn't as important to me, but I prefer for the
> clockdomain to not be a clock provider if possible.

Yeah I totally agree with that, and that's already what we mostly
have.

> > I don't at least yet
> > follow what we need to do with the clockdomains with genpd :)
> 
> Use the clockdomain genpd to call clk_enable/disable under the hood.
> Don't use them as clock providers to the target modules. Clockdomain
> genpds would be the clock consumers.

I don't think the clockdomain should be a genpd provider because
that creates a genpd network of dependencies instead of a tree
structure. If we end up setting the clockdomains with genpd, then
only the other clockdomains should use them, but I don't know how
we ever keep drivers from directly tinkering with them..

IMO, the clockdomain clock driver should just provides clocks, then
we can have the interconnect target module driver deal with the
clockdomain dependencies.

> > Wouldn't just doing clk_get() from one clockdomain clock to
> > another clockdomain clock (or it's output) be enough to
> > represent the clockdomain dependencies?
> 
> s/clk_get/clk_prepare_enable/
> 
> Yes, but you're stuffing functional dependencies into the clock tree,
> which sucks. genpd was created to model these arbitrary dependencies.

Well let's not stuff anything beyond clock framework to the
clockdomain clock drivers. We already have the clockdomain
dependencies handled by the interconnect code (hwmod), and there
should be no problem moving those to be handled by genpd and the
interconnect target driver instances.

Care to take another look at Tero's patches with the assumption
that the clockdomain clocks stay just as a clocks?

Regards,

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

^ permalink raw reply

* Re: [PATCH 1/3] clkdev: add devm_get_clk_from_child()
From: Russell King - ARM Linux @ 2016-12-09 20:31 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Stephen Boyd, Rob Herring, Linux-ALSA, Linux-DT,
	Michael Turquette, Linux-Kernel, Mark Brown, linux-clk, Linux-ARM
In-Reply-To: <8737i3vtl7.wl%kuninori.morimoto.gx@renesas.com>

On Mon, Dec 05, 2016 at 05:23:20AM +0000, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Some driver is using this type of DT bindings for clock (more detail,
> see ${LINUX}/Documentation/devicetree/bindings/sound/simple-card.txt).
> 
> 	sound_soc {
> 		...
> 		cpu {
> 			clocks = <&xxx>;
> 			...
> 		};
> 		codec {
> 			clocks = <&xxx>;
> 			...
> 		};
> 	};
> 
> Current driver in this case uses of_clk_get() for each node, but there
> is no devm_of_clk_get() today.
> OTOH, the problem of having devm_of_clk_get() is that it encourages the
> use of of_clk_get() when clk_get() is more desirable.
> 
> Thus, this patch adds new devm_get_clk_from_chile() which explicitly
> reads as get a clock from a child node of this device.
> By this function, we can also use this type of DT bindings
> 
> 	sound_soc {
> 		clocks = <&xxx>, <&xxx>;
> 		clock-names = "cpu", "codec";
> 		...
> 		cpu {
> 			...
> 		};
> 		codec {
> 			...
> 		};
> 	};
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

This looks lots better, thanks.

Acked-by: Russell King <rmk+kernel@armlinux.org.uk>

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* Re: [PATCHv4 11/15] clk: ti: clockdomain: add clock provider support to clockdomains
From: Michael Turquette @ 2016-12-09 20:02 UTC (permalink / raw)
  To: Tony Lindgren, Tero Kristo
  Cc: Stephen Boyd, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring
In-Reply-To: <20161205152534.GJ4705-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

Quoting Tony Lindgren (2016-12-05 07:25:34)
> * Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org> [161205 02:09]:
> > On 03/12/16 02:18, Tony Lindgren wrote:
> > > * Michael Turquette <mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> [161202 15:52]:
> > > > Quoting Tony Lindgren (2016-12-02 15:12:40)
> > > > > * Michael Turquette <mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> [161202 14:34]:
> > > > > > Quoting Tony Lindgren (2016-10-28 16:54:48)
> > > > > > > * Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> [161028 16:37]:
> > > > > > > > On 10/28, Tony Lindgren wrote:
> > > > > > > > > * Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org> [161028 00:43]:
> > > > > > > > > > On 28/10/16 03:50, Stephen Boyd wrote:
> > > > > > > > > > > I suppose a PRCM is
> > > > > > > > > > > like an MFD that has clocks and resets under it? On other
> > > > > > > > > > > platforms we've combined that all into one node and just had
> > > > > > > > > > > #clock-cells and #reset-cells in that node. Is there any reason
> > > > > > > > > > > we can't do that here?
> > > > > > > > > > 
> > > > > > > > > > For OMAPs, there are typically multiple instances of the PRCM around; OMAP4
> > > > > > > > > > for example has:
> > > > > > > > > > 
> > > > > > > > > > cm1 @ 0x4a004000 (clocks + clockdomains)
> > > > > > > > > > cm2 @ 0x4a008000 (clocks + clockdomains)
> > > > > > > > > > prm @ 0x4a306000 (few clocks + resets + power state handling)
> > > > > > > > > > scrm @ 0x4a30a000 (few external clocks + plenty of misc stuff)
> > > > > > > > > > 
> > > > > > > > > > These instances are also under different power/voltage domains which means
> > > > > > > > > > their PM behavior is different.
> > > > > > > > > > 
> > > > > > > > > > The idea behind having a clockdomain as a provider was mostly to have the
> > > > > > > > > > topology visible : prcm-instance -> clockdomain -> clocks
> > > > > > > > > 
> > > > > > > > > Yeah that's needed to get the interconnect hierarchy right for
> > > > > > > > > genpd :)
> > > > > > > > > 
> > > > > > > > > > ... but basically I think it would be possible to drop the clockdomain
> > > > > > > > > > representation and just mark the prcm-instance as a clock provider. Tony,
> > > > > > > > > > any thoughts on that?
> > > > > > > > > 
> > > > > > > > > No let's not drop the clockdomains as those will be needed when we
> > > > > > > > > move things into proper hierarchy within the interconnect instances.
> > > > > > > > > This will then help with getting things right with genpd.
> > > > > > > > > 
> > > > > > > > > In the long run we just want to specify clockdomain and the offset of
> > > > > > > > > the clock instance within the clockdomain in the dts files.
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > Sorry, I have very little idea how OMAP hardware works. Do you
> > > > > > > > mean that you will have different nodes for each clockdomain so
> > > > > > > > that genpd can map 1:1 to the node in dts? But in hardware
> > > > > > > > there's a prcm that allows us to control many clock domains
> > > > > > > > through register read/writes? How is the interconnect involved?
> > > > > > > 
> > > > > > > There are multiple clockdomains, at least one for each interconnect
> > > > > > > instance. Once a clockdomain is idle, the related interconnect can
> > > > > > > idle too. So yeah genpd pretty much maps 1:1 with the clockdomains.
> > > > > > > 
> > > > > > > There's more info in for example omap4 TRM section "3.4.1 Device
> > > > > > > Power-Management Layout" that shows the voltage/power/clock domains.
> > > > > > > The interconnect instances are mostly named there too looking at
> > > > > > > the L4/L3 naming.
> > > > > > 
> > > > > > I'm confused on two points:
> > > > > > 
> > > > > > 1) why are the clkdm's acting as clock providers? I've always hated the
> > > > > > name "clock domain" since those bits are for managing module state, not
> > > > > > clock state. The PRM, CM1 and CM2 provide the clocks, not the
> > > > > > clockdomains.
> > > > > 
> > > > > The clock domains have multiple clock inputs that are routed to multiple
> > > > > child clocks. So it is a clock :)
> > > > > 
> > > > > See for example omap4430 TRM "3.6.4 CD_WKUP Clock Domain" on page
> > > > > 393 in my revision here.
> > > > > 
> > > > > On that page "Figure 3-48" shows CD_WKUP with the four input clocks.
> > > > > And then "Table 3-84. CD_WKUP Control and Status Parameters" shows
> > > > > the CD_WKUP clock domain specific registers. These registers show
> > > > > the status, I think they are all read-only registers. Then CD_WKUP
> > > > > has multiple child clocks with configurable registers.
> > > > > 
> > > > > From hardware register point of view, each clock domain has:
> > > > > 
> > > > > - Read-only clockdomain status registers in the beginning of
> > > > >   the address space
> > > > > 
> > > > > - Multiple similar clock instances register instances each
> > > > >   mapping to a specific interconnect target module
> > > > > 
> > > > > These are documented in "3.11.16.1 WKUP_CM Register Summary".
> > > > 
> > > > Oh, this is because you are treating the MODULEMODE bits like gate
> > > > clocks. I never really figured out if this was the best way to model
> > > > those bits since they do more than control a line toggling at a rate.
> > > > For instance this bit will affect the master/slave IDLE protocol between
> > > > the module and the PRCM.
> > > 
> > > Yes seems like there is some negotiation going on there with the
> > > target module. But from practical point of view the CLKCTRL
> > > register is the gate for a module functional clock.
> > 
> > There's some confusion on this, clockdomain is effectively a collection of
> > clocks, and can be used to force control that collection if needed. Chapter
> > "3.1.1.1.3 Clock Domain" in some OMAP4 TRM shows the relationship neatly.
> 
> Yeah that's my understanding too.

There is no clk api for this type of behavior. We keep clocks enabled so
long as consumers have a positive prepare_count or enable_count. The
concept of force-idle doesn't work very well here, unless any calls to
force the clkdm to idle also use a usecount.

> 
> > > > > From hardware point of view, we ideally want to map interconnect
> > > > > target modules to the clock instance offset from the clock domain
> > > > > for that interconnect segment. For example gptimer1 clocks would
> > > > > be just:
> > > > > 
> > > > > clocks = <&cd_wkup 0x40>;
> > > > > 
> > > > > > 2) why aren't the clock domains modeled as genpds with their associated
> > > > > > devices attached to them? Note that it is possible to "nest" genpd
> > > > > > objects. This would also allow for the "Clockdomain Dependency"
> > > > > > relationships to be properly modeled (see section 3.1.1.1.7 Clock Domain
> > > > > > Dependency in the OMAP4 TRM).
> > > > > 
> > > > > Clock domains only route clocks to child clocks. Power domains
> > > > > are different registers. The power domains map roughly to
> > > > > interconnect instances, there we have registers to disable the
> > > > > whole interconnect when idle.
> > > > 
> > > > I'm not talking about power islands at all, but the genpd object in
> > > > Linux. For instance, if we treat each clock domain like a clock
> > > > provider, how could the functional dependency between clkdm_A and
> > > > clkdm_B be asserted?
> > > 
> > > To me it seems that some output of a clockdomain is just a input
> > > of another clockdomain? So it's just the usual parent child
> > > relationship once we treat a clockdomain just as a clock. Tero
> > > probably has some input here.
> > 
> > A clockdomain should be modelled as a genpd, that I agree. However, it
> > doesn't prevent it from being a clock provider also, or does it?

It does not prevent it, but I don't understand why you would want both.

I had a recent conversation with Kevin Hilman about a related issue
(we were not discussing this thread or this series) and we both agreed
that most drivers don't even need to managed their clocks directly, so
much as they need to manage their on/off resources. Clocks are just one
part of that, and if we can hide that stuff inside of an attached genpd
then it would be better than having the driver manage clocks explicitly.

Obviously some devices such as audio codec or uart will need to manage
clocks directly, but this is mostly the exception, not the rule.

> > 
> > > > There is certainly no API for that in the clock framework, but for genpd
> > > > your runtime_pm_get() callback for clkdm_A could call runtime_pm_get
> > > > against clkdm_B, which would satisfy the requirement. See section
> > > > 3.1.1.1.7 Clock Domain Dependency in the OMAP4 TRM, version AB.
> > 
> > For static dependencies the apis genpd_add/remove_subdomain could probably
> > be used.
> > 
> > > To me it seems the API is just clk_get() :) Do you have some
> > > specific example we can use to check? My guess is that the
> > > TRM "Clock Domain Dependency" is just the usual parent child
> > > relationship between clocks that are the clockdomains..

clk_get() only fetches a pointer to the clk. I guess you mean
clk_prepare_enable() to actually increment the use count?

If we used the clk framework here is that it would look something like
this:

clk_enable(clk_a)
-> .enable(clk_a_hw)
   -> clk_enable(clk_b)

However, clk_a and clk_b do not have a parent-child relationship in the
clock tree. This is purely a functional relationship between IP blocks.
Modeling this sort of thing in the clk framework would be wrong, and
genpd is a much better place to establish these arbitrary relationships.

> > > 
> > > If there is something more magical there certainly that should
> > > be considered though.
> > 
> > The hwmods could be transformed to individual genpds also I guess. On DT
> > level though, we would still need a clock pointer to the main clock and a
> > genpd pointer in addition to that.
> 
> Hmm a genpd pointer to where exactly? AFAIK each interconnect
> instance should be a genpd provider, and the individual interconnect
> target modules should be consumers for that genpd.

I was thinking that the clock domains would be modeled as genpd objects
with the interconnect target modules attached as struct devices.

> 
> > Tony, any thoughts on that? Would this break up the plans for the
> > interconnect completely?
> 
> Does using genpd for clockdomains cause issues for using genpd for
> interconnect instances and the target modules?

Can they be the same object in Linux? If there is a one-to-one mapping
between clock domains and the interconnect port then maybe you can just
model them together.

> 
> The thing I'd be worried about there is that the clockdomains and
> their child clocks are just devices sitting on the interconnect,
> so we could easily end up with genpd modeling something that does
> not represent the hardware.
> 
> For example, on 4430 we have:
> 
> l4_cfg interconnect
>        ...
>        segment@0
>                 ...
>                 target_module@4000
>                         cm1: cm1@0

How about:

l4_cfg interconnect
       ...
       segment@0
                ...
                cm1@4000
                	module: foo_module@0

I don't know much about the segments. Do they map one-to-one with the
clock domains?

>                              ...
>                 ...
>                 target_module@8000
>                         cm2: cm2@0
>                 ...
> 
> 
> l4_wkup interonnect
>         ...
>         segment@0
>                 ...
>                 target_module@6000
>                         prm: prm@0
>                 ...
>                 target_module@a000
>                         scrm: scrm@0
>                 ...
> 
> So what do you guys have in mind for using genpd in the above
> example for the clockdomains?

If my quick-and-dirty DT above makes sense, then the target modules
(e.g. io controller) would not get clocks anymore, but just
pm_runtime_get(). The genpd backing object would call clk_enable/disable
as needed.

If fine grained control of a clock is needed (e.g. for clk_set_rate)
then the driver can still clk_get it. Whether or not the clockdomain
provides that clock or if it comes from the clock generator (e.g. cm1,
cm2, prm, etc) isn't as important to me, but I prefer for the
clockdomain to not be a clock provider if possible.

> 
> To me it seems that the interconnect instances like l4_cfg and
> l4_wkup above should be genpd providers.

Agreed.

> I don't at least yet
> follow what we need to do with the clockdomains with genpd :)

Use the clockdomain genpd to call clk_enable/disable under the hood.
Don't use them as clock providers to the target modules. Clockdomain
genpds would be the clock consumers.

> 
> Wouldn't just doing clk_get() from one clockdomain clock to
> another clockdomain clock (or it's output) be enough to
> represent the clockdomain dependencies?

s/clk_get/clk_prepare_enable/

Yes, but you're stuffing functional dependencies into the clock tree,
which sucks. genpd was created to model these arbitrary dependencies.

Regards,
Mike

> 
> Regards,
> 
> Tony
--
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 2/2] usb: gadget: s3c2410: allow probing from device tree
From: Sergio Prado @ 2016-12-09 19:06 UTC (permalink / raw)
  To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	balbi-DgEjT+Ai2ygdnm+yROfE0A, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Sergio Prado
In-Reply-To: <1481310400-28676-1-git-send-email-sergio.prado-1e4yhPs3/ABSwrhanM7KvQ@public.gmane.org>

Allows configuring Samsung's s3c2410 and compatible USB device
controller using a devicetree.

Signed-off-by: Sergio Prado <sergio.prado-1e4yhPs3/ABSwrhanM7KvQ@public.gmane.org>
---
 drivers/usb/gadget/udc/s3c2410_udc.c | 142 +++++++++++++++++++++++++++++------
 drivers/usb/gadget/udc/s3c2410_udc.h |   4 +
 2 files changed, 123 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/gadget/udc/s3c2410_udc.c b/drivers/usb/gadget/udc/s3c2410_udc.c
index 4643a01262b4..e3b5a0e6646e 100644
--- a/drivers/usb/gadget/udc/s3c2410_udc.c
+++ b/drivers/usb/gadget/udc/s3c2410_udc.c
@@ -30,6 +30,9 @@
 #include <linux/gpio.h>
 #include <linux/prefetch.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
 
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
@@ -55,6 +58,18 @@
 #define DRIVER_AUTHOR	"Herbert Pötzl <herbert-dBHVzrDq9nF4Lj/PQRBjDg@public.gmane.org>, " \
 			"Arnaud Patard <arnaud.patard-dQbF7i+pzddAfugRpC6u6w@public.gmane.org>"
 
+struct s3c2410_udc_drv_data {
+	int ep_fifo_size;
+};
+
+static const struct s3c2410_udc_drv_data s3c2410_udc_2410_drv_data = {
+	.ep_fifo_size = EP_FIFO_SIZE,
+};
+
+static const struct s3c2410_udc_drv_data s3c2410_udc_2440_drv_data = {
+	.ep_fifo_size = S3C2440_EP_FIFO_SIZE,
+};
+
 static const char		gadget_name[] = "s3c2410_udc";
 static const char		driver_desc[] = DRIVER_DESC;
 
@@ -62,8 +77,6 @@
 static struct clk		*udc_clock;
 static struct clk		*usb_bus_clock;
 static void __iomem		*base_addr;
-static u64			rsrc_start;
-static u64			rsrc_len;
 static struct dentry		*s3c2410_udc_debugfs_root;
 
 static inline u32 udc_read(u32 reg)
@@ -997,7 +1010,7 @@ static irqreturn_t s3c2410_udc_irq(int dummy, void *_dev)
 		}
 	}
 
-	dprintk(DEBUG_VERBOSE, "irq: %d s3c2410_udc_done.\n", IRQ_USBD);
+	dprintk(DEBUG_VERBOSE, "irq: %d s3c2410_udc_done.\n", dev->irq);
 
 	/* Restore old index */
 	udc_write(idx, S3C2410_UDC_INDEX_REG);
@@ -1757,6 +1770,49 @@ static int s3c2410_udc_stop(struct usb_gadget *g)
 
 };
 
+static int s3c2410_udc_probe_dt(struct s3c2410_udc *udc)
+{
+	const struct s3c2410_udc_drv_data *drvdata;
+	struct platform_device *pdev = udc->pdev;
+	struct s3c2410_udc_mach_info *pdata;
+	int gpio;
+
+	drvdata = of_device_get_match_data(&pdev->dev);
+
+	if (drvdata)
+		udc->ep_fifo_size = drvdata->ep_fifo_size;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	gpio = of_get_named_gpio(pdev->dev.of_node, "samsung,vbus-gpio", 0);
+	if (gpio_is_valid(gpio))
+		pdata->vbus_pin = gpio;
+
+	gpio = of_get_named_gpio(pdev->dev.of_node, "samsung,pullup-gpio", 0);
+	if (gpio_is_valid(gpio))
+		pdata->pullup_pin = gpio;
+
+	pdev->dev.platform_data = pdata;
+
+	return 0;
+}
+
+static int s3c2410_udc_probe_pdata(struct s3c2410_udc *udc)
+{
+	const struct s3c2410_udc_drv_data *drvdata;
+	struct platform_device *pdev = udc->pdev;
+
+	drvdata = (struct s3c2410_udc_drv_data *)
+		platform_get_device_id(pdev)->driver_data;
+
+	if (drvdata)
+		udc->ep_fifo_size = drvdata->ep_fifo_size;
+
+	return 0;
+}
+
 /*
  *	probe - binds to the platform device
  */
@@ -1769,6 +1825,16 @@ static int s3c2410_udc_probe(struct platform_device *pdev)
 
 	dev_dbg(dev, "%s()\n", __func__);
 
+	udc->pdev = pdev;
+
+	if (pdev->dev.of_node)
+		retval = s3c2410_udc_probe_dt(udc);
+	else
+		retval = s3c2410_udc_probe_pdata(udc);
+
+	if (retval)
+		return retval;
+
 	usb_bus_clock = clk_get(NULL, "usb-bus-gadget");
 	if (IS_ERR(usb_bus_clock)) {
 		dev_err(dev, "failed to get usb bus clock source\n");
@@ -1789,24 +1855,27 @@ static int s3c2410_udc_probe(struct platform_device *pdev)
 
 	dev_dbg(dev, "got and enabled clocks\n");
 
-	if (strncmp(pdev->name, "s3c2440", 7) == 0) {
-		dev_info(dev, "S3C2440: increasing FIFO to 128 bytes\n");
-		memory.ep[1].fifo_size = S3C2440_EP_FIFO_SIZE;
-		memory.ep[2].fifo_size = S3C2440_EP_FIFO_SIZE;
-		memory.ep[3].fifo_size = S3C2440_EP_FIFO_SIZE;
-		memory.ep[4].fifo_size = S3C2440_EP_FIFO_SIZE;
+	if (udc->ep_fifo_size) {
+		dev_info(dev, "setting FIFO to %d bytes\n", udc->ep_fifo_size);
+		memory.ep[1].fifo_size = udc->ep_fifo_size;
+		memory.ep[2].fifo_size = udc->ep_fifo_size;
+		memory.ep[3].fifo_size = udc->ep_fifo_size;
+		memory.ep[4].fifo_size = udc->ep_fifo_size;
 	}
 
 	spin_lock_init(&udc->lock);
 	udc_info = dev_get_platdata(&pdev->dev);
 
-	rsrc_start = S3C2410_PA_USBDEV;
-	rsrc_len   = S3C24XX_SZ_USBDEV;
+	udc->mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!udc->mem) {
+		dev_err(dev, "failed to get I/O memory region\n");
+		return -ENOENT;
+	}
 
-	if (!request_mem_region(rsrc_start, rsrc_len, gadget_name))
+	if (!request_mem_region(udc->mem->start, resource_size(udc->mem), gadget_name))
 		return -EBUSY;
 
-	base_addr = ioremap(rsrc_start, rsrc_len);
+	base_addr = ioremap(udc->mem->start, resource_size(udc->mem));
 	if (!base_addr) {
 		retval = -ENOMEM;
 		goto err_mem;
@@ -1818,17 +1887,24 @@ static int s3c2410_udc_probe(struct platform_device *pdev)
 	s3c2410_udc_disable(udc);
 	s3c2410_udc_reinit(udc);
 
+	udc->irq = platform_get_irq(pdev, 0);
+	if (udc->irq == 0) {
+		dev_err(dev, "failed to get interrupt\n");
+		retval = -EINVAL;
+		goto err_map;
+	}
+
 	/* irq setup after old hardware state is cleaned up */
-	retval = request_irq(IRQ_USBD, s3c2410_udc_irq,
+	retval = request_irq(udc->irq, s3c2410_udc_irq,
 			     0, gadget_name, udc);
 
 	if (retval != 0) {
-		dev_err(dev, "cannot get irq %i, err %d\n", IRQ_USBD, retval);
+		dev_err(dev, "cannot get irq %i, err %d\n", udc->irq, retval);
 		retval = -EBUSY;
 		goto err_map;
 	}
 
-	dev_dbg(dev, "got irq %i\n", IRQ_USBD);
+	dev_dbg(dev, "got irq %i\n", udc->irq);
 
 	if (udc_info && udc_info->vbus_pin > 0) {
 		retval = gpio_request(udc_info->vbus_pin, "udc vbus");
@@ -1899,11 +1975,11 @@ static int s3c2410_udc_probe(struct platform_device *pdev)
 	if (udc_info && udc_info->vbus_pin > 0)
 		gpio_free(udc_info->vbus_pin);
 err_int:
-	free_irq(IRQ_USBD, udc);
+	free_irq(udc->irq, udc);
 err_map:
 	iounmap(base_addr);
 err_mem:
-	release_mem_region(rsrc_start, rsrc_len);
+	release_mem_region(udc->mem->start, resource_size(udc->mem));
 
 	return retval;
 }
@@ -1933,10 +2009,10 @@ static int s3c2410_udc_remove(struct platform_device *pdev)
 		free_irq(irq, udc);
 	}
 
-	free_irq(IRQ_USBD, udc);
+	free_irq(udc->irq, udc);
 
 	iounmap(base_addr);
-	release_mem_region(rsrc_start, rsrc_len);
+	release_mem_region(udc->mem->start, resource_size(udc->mem));
 
 	if (!IS_ERR(udc_clock) && udc_clock != NULL) {
 		clk_disable_unprepare(udc_clock);
@@ -1974,16 +2050,36 @@ static int s3c2410_udc_resume(struct platform_device *pdev)
 #define s3c2410_udc_resume	NULL
 #endif
 
+static const struct of_device_id s3c_udc_dt_ids[] = {
+	{
+		.compatible = "samsung,s3c2410-udc",
+		.data = &s3c2410_udc_2410_drv_data,
+	},
+	{
+		.compatible = "samsung,s3c2440-udc",
+		.data = &s3c2410_udc_2440_drv_data,
+	},
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, s3c_udc_dt_ids);
+
 static const struct platform_device_id s3c_udc_ids[] = {
-	{ "s3c2410-usbgadget", },
-	{ "s3c2440-usbgadget", },
-	{ }
+	{
+		.name = "s3c2410-usbgadget",
+		.driver_data = (kernel_ulong_t) &s3c2410_udc_2410_drv_data,
+	},
+	{
+		.name = "s3c2440-usbgadget",
+		.driver_data = (kernel_ulong_t) &s3c2410_udc_2440_drv_data,
+	},
+	{ /* sentinel */},
 };
 MODULE_DEVICE_TABLE(platform, s3c_udc_ids);
 
 static struct platform_driver udc_driver_24x0 = {
 	.driver		= {
 		.name	= "s3c24x0-usbgadget",
+		.of_match_table = s3c_udc_dt_ids,
 	},
 	.probe		= s3c2410_udc_probe,
 	.remove		= s3c2410_udc_remove,
diff --git a/drivers/usb/gadget/udc/s3c2410_udc.h b/drivers/usb/gadget/udc/s3c2410_udc.h
index 93bf225f1969..8fdbe77a804d 100644
--- a/drivers/usb/gadget/udc/s3c2410_udc.h
+++ b/drivers/usb/gadget/udc/s3c2410_udc.h
@@ -94,6 +94,10 @@ struct s3c2410_udc {
 	unsigned			req_pending : 1;
 	u8				vbus;
 	struct dentry			*regs_info;
+	struct platform_device		*pdev;
+	struct resource			*mem;
+	int				irq;
+	unsigned int			ep_fifo_size;
 };
 #define to_s3c2410(g)	(container_of((g), struct s3c2410_udc, gadget))
 
-- 
1.9.1

--
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 1/2] dt-bindings: usb: add DT binding for s3c2410 USB device controller
From: Sergio Prado @ 2016-12-09 19:06 UTC (permalink / raw)
  To: gregkh, robh+dt, mark.rutland, balbi, linux-usb, devicetree,
	linux-kernel
  Cc: Sergio Prado
In-Reply-To: <1481310400-28676-1-git-send-email-sergio.prado@e-labworks.com>

Adds the device tree bindings description for Samsung S3C2410 and
compatible USB device controller.

Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
---
 .../devicetree/bindings/usb/s3c2410-usb.txt        | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/s3c2410-usb.txt b/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
index e45b38ce2986..4d3f9894c2d4 100644
--- a/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
+++ b/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
@@ -20,3 +20,31 @@ usb0: ohci@49000000 {
 	clocks = <&clocks UCLK>, <&clocks HCLK_USBH>;
 	clock-names = "usb-bus-host", "usb-host";
 };
+
+Samsung S3C2410 and compatible USB device controller
+
+Required properties:
+ - compatible: Should be one of the following
+	       "samsung,s3c2410-udc"
+	       "samsung,s3c2440-udc"
+ - reg: address and length of the controller memory mapped region
+ - interrupts: interrupt number for the USB device controller
+ - clocks: Should reference the bus and host clocks
+ - clock-names: Should contain two strings
+		"usb-bus-gadget" for the USB bus clock
+		"usb-device" for the USB device clock
+
+Optional properties:
+ - samsung,vbus-gpio: If present, specifies a gpio that needs to be
+   activated for the bus to be powered.
+ - samsung,pullup-gpio: If present, specifies a gpio to control the
+   USB D+ pullup.
+
+usb1: udc@52000000 {
+	compatible = "samsung,s3c2440-udc";
+	reg = <0x52000000 0x100000>;
+	interrupts = <0 0 25 3>;
+	clocks = <&clocks UCLK>, <&clocks HCLK_USBD>;
+	clock-names = "usb-bus-gadget", "usb-device";
+	samsung,pullup-gpio = <&gpc 5 GPIO_ACTIVE_HIGH>;
+};
-- 
1.9.1

^ permalink raw reply related

* [PATCH 0/2] usb: gadget: s3c2410: add device tree support
From: Sergio Prado @ 2016-12-09 19:06 UTC (permalink / raw)
  To: gregkh, robh+dt, mark.rutland, balbi, linux-usb, devicetree,
	linux-kernel
  Cc: Sergio Prado

This series adds support for configuring Samsung's s3c2410 and
compatible USB device controller via devicetree.

Tested on FriendlyARM mini2440, based on s3c2440 SoC.

Sergio Prado (2):
  dt-bindings: usb: add DT binding for s3c2410 USB device controller
  usb: gadget: s3c2410: allow probing from device tree

 .../devicetree/bindings/usb/s3c2410-usb.txt        |  28 ++++
 drivers/usb/gadget/udc/s3c2410_udc.c               | 142 +++++++++++++++++----
 drivers/usb/gadget/udc/s3c2410_udc.h               |   4 +
 3 files changed, 151 insertions(+), 23 deletions(-)

-- 
1.9.1

^ permalink raw reply

* Re: [PATCH 6/9] dt-bindings: Document rk3399 Gru/Kevin
From: Heiko Stuebner @ 2016-12-09 18:28 UTC (permalink / raw)
  To: Rob Herring
  Cc: Brian Norris, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Caesar Wang, Doug Anderson,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Stephen Barber,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Chris Zhong
In-Reply-To: <20161209175402.gy4mqjaf2rsib7qf@rob-hp-laptop>

Am Freitag, 9. Dezember 2016, 11:54:02 CET schrieb Rob Herring:
> On Thu, Dec 01, 2016 at 06:27:30PM -0800, Brian Norris wrote:
> > Gru is a base dev board for a family of devices, including Kevin. Both
> > utilize Rockchip RK3399, and they share much of their design.
> > 
> > Signed-off-by: Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > ---
> > 
> >  Documentation/devicetree/bindings/arm/rockchip.txt | 20
> >  ++++++++++++++++++++ 1 file changed, 20 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/rockchip.txt
> > b/Documentation/devicetree/bindings/arm/rockchip.txt index
> > cc4ace6397ab..830e13f5890c 100644
> > --- a/Documentation/devicetree/bindings/arm/rockchip.txt
> > +++ b/Documentation/devicetree/bindings/arm/rockchip.txt
> > @@ -99,6 +99,26 @@ Rockchip platforms device tree bindings
> > 
> >  		     "google,veyron-speedy-rev3", "google,veyron-speedy-rev2",
> >  		     "google,veyron-speedy", "google,veyron", "rockchip,rk3288";
> > 
> > +- Google Gru (dev-board):
> > +    Required root node properties:
> > +      - compatible = "google,gru-rev15", "google,gru-rev14",
> > +		     "google,gru-rev13", "google,gru-rev12",
> > +		     "google,gru-rev11", "google,gru-rev10",
> > +		     "google,gru-rev9", "google,gru-rev8",
> > +		     "google,gru-rev7", "google,gru-rev6",
> > +		     "google,gru-rev5", "google,gru-rev4",
> > +		     "google,gru-rev3", "google,gru-rev2",
> > +		     "google,gru", "rockchip,rk3399";
> 
> All of these are supposed to be specified or just one rev at a time?

All of them. I.e. the devicetree is supposed to be compatible with all of 
those, while the bootloader determines the actual board revision and sets the 
choosen compatible - at least that was the way with the previous series of 
devices based around the rk3288 (veyron) and I think it will be the same way 
still.

I.e. as you can see below, Kevin starts being compatible at -rev6, as 
(engineering) revisions before that probably had some differences on the 
board.


> 
> > +
> > +- Google Kevin:
> > +    Required root node properties:
> > +      - compatible = "google,kevin-rev15", "google,kevin-rev14",
> > +		     "google,kevin-rev13", "google,kevin-rev12",
> > +		     "google,kevin-rev11", "google,kevin-rev10",
> > +		     "google,kevin-rev9", "google,kevin-rev8",
> > +		     "google,kevin-rev7", "google,kevin-rev6",
> > +		     "google,kevin", "google,gru", "rockchip,rk3399";
> > +
> > 
> >  - mqmaker MiQi:
> >      Required root node properties:
> >        - compatible = "mqmaker,miqi", "rockchip,rk3288";


--
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: Guilherme G. Piccoli @ 2016-12-09 18:19 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
	linuxppc-dev, Mark Rutland, Benjamin Herrenschmidt, Frank Rowand,
	Marc Zyngier
In-Reply-To: <CAL_JsqJQEYmxbUaL5qhym1MHRvUbo=PvUWDMKRDqLnBhuJAYzQ@mail.gmail.com>

On 12/09/2016 02:25 PM, Rob Herring wrote:
> On Mon, Dec 5, 2016 at 1:01 PM, Guilherme G. Piccoli
> <gpiccoli@linux.vnet.ibm.com> wrote:
>> On 12/05/2016 12:28 PM, Rob Herring wrote:
>>> On Mon, Dec 5, 2016 at 7:59 AM, Guilherme G. Piccoli
>>> <gpiccoli@linux.vnet.ibm.com> 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@linux.vnet.ibm.com>
>>>> ---
>>>>
>>>> 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.
> 

OK, perfect! Thanks Rob
Cheers,


Guilherme

> Rob
> 

^ permalink raw reply

* Re: [PATCH 2/2] drm/panel: simple: Add support for Tianma TM070JDHG30
From: Rob Herring @ 2016-12-09 18:14 UTC (permalink / raw)
  To: Gary Bisson; +Cc: devicetree, linux-kernel, dri-devel
In-Reply-To: <20161202085208.19459-3-gary.bisson@boundarydevices.com>

On Fri, Dec 02, 2016 at 09:52:08AM +0100, Gary Bisson wrote:
> The Tianma TM070JDHG30 is a 7" LVDS display with a resolution of
> 1280x800.
> http://usa.tianma.com/products-technology/product/tm070jdhg30-00
> 
> You can also find this product along with a FT5x06 touch controller
> from Boundary Devices:
> https://boundarydevices.com/product/bd070lic2/
> 
> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
> ---
>  .../bindings/display/panel/tianma,tm070jdhg30.txt  |  7 ++++++

Acked-by: Rob Herring <robh@kernel.org> 

>  drivers/gpu/drm/panel/panel-simple.c               | 27 ++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/tianma,tm070jdhg30.txt
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH 1/2] of: Add vendor prefix for Tianma Micro-electronics
From: Rob Herring @ 2016-12-09 18:12 UTC (permalink / raw)
  To: Gary Bisson; +Cc: devicetree, linux-kernel, dri-devel
In-Reply-To: <20161202085208.19459-2-gary.bisson@boundarydevices.com>

On Fri, Dec 02, 2016 at 09:52:07AM +0100, Gary Bisson wrote:
> Tianma Micro-electronics Co., Ltd. (Tianma) specializes in providing
> display solutions and efficient support services worldwide.
> 
> More info:
> http://en.tianma.com/about.shtml
> 
> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Rob Herring <robh@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH 10/11] Document: dt: binding: imx: update doc for imx6sll
From: Rob Herring @ 2016-12-09 18:12 UTC (permalink / raw)
  To: Bai Ping
  Cc: shawnguo-DgEjT+Ai2ygdnm+yROfE0A,
	mturquette-rdvid1DuHRBWk0Htik3J/w, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
	mark.rutland-5wv7dgnIgG8, linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA, p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ,
	fabio.estevam-3arQi8VN3Tc, tglx-hfZtesqFncYOwBW4kG4KsQ,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1480660774-25055-11-git-send-email-ping.bai-3arQi8VN3Tc@public.gmane.org>

On Fri, Dec 02, 2016 at 02:39:33PM +0800, Bai Ping wrote:
> Add necessary document update for i.MX6SLL support.
> 
> Signed-off-by: Bai Ping <ping.bai-3arQi8VN3Tc@public.gmane.org>
> ---
>  .../devicetree/bindings/clock/imx6sll-clock.txt    | 13 ++++++++
>  .../bindings/pinctrl/fsl,imx6sll-pinctrl.txt       | 37 ++++++++++++++++++++++
>  2 files changed, 50 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/imx6sll-clock.txt
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/fsl,imx6sll-pinctrl.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/imx6sll-clock.txt b/Documentation/devicetree/bindings/clock/imx6sll-clock.txt
> new file mode 100644
> index 0000000..4f52efa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/imx6sll-clock.txt
> @@ -0,0 +1,13 @@
> +* Clock bindings for Freescale i.MX6 UltraLite

I thought UltraLite was MX6UL?

> +
> +Required properties:
> +- compatible: Should be "fsl,imx6sll-ccm"
> +- reg: Address and length of the register set
> +- #clock-cells: Should be <1>
> +- clocks: list of clock specifiers, must contain an entry for each required
> +  entry in clock-names
> +- clock-names: should include entries "ckil", "osc", "ipp_di0" and "ipp_di1"
> +
> +The clock consumer should specify the desired clock by having the clock
> +ID in its "clocks" phandle cell.  See include/dt-bindings/clock/imx6sll-clock.h
> +for the full list of i.MX6 SLL clock IDs.
> diff --git a/Documentation/devicetree/bindings/pinctrl/fsl,imx6sll-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/fsl,imx6sll-pinctrl.txt
> new file mode 100644
> index 0000000..096e471
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/fsl,imx6sll-pinctrl.txt
> @@ -0,0 +1,37 @@
> +* Freescale i.MX6 UltraLite IOMUX Controller

ditto

> +
> +Please refer to fsl,imx-pinctrl.txt in this directory for common binding part
> +and usage.
> +
> +Required properties:
> +- compatible: "fsl,imx6sll-iomuxc"
> +- fsl,pins: each entry consists of 6 integers and represents the mux and config
> +  setting for one pin.  The first 5 integers <mux_reg conf_reg input_reg mux_val
> +  input_val> are specified using a PIN_FUNC_ID macro, which can be found in
> +  imx6ul-pinfunc.h under device tree source folder.  The last integer CONFIG is
> +  the pad setting value like pull-up on this pin.  Please refer to i.MX6SLL
> +  Reference Manual for detailed CONFIG settings.
> +
> +CONFIG bits definition:
> +PAD_CTL_LVE			(1 << 22)
> +PAD_CTL_HYS                     (1 << 16)
> +PAD_CTL_PUS_100K_DOWN           (0 << 14)
> +PAD_CTL_PUS_47K_UP              (1 << 14)
> +PAD_CTL_PUS_100K_UP             (2 << 14)
> +PAD_CTL_PUS_22K_UP              (3 << 14)
> +PAD_CTL_PUE                     (1 << 13)
> +PAD_CTL_PKE                     (1 << 12)
> +PAD_CTL_ODE                     (1 << 11)
> +PAD_CTL_SPEED_LOW               (0 << 6)
> +PAD_CTL_SPEED_MED               (1 << 6)
> +PAD_CTL_SPEED_HIGH              (3 << 6)
> +PAD_CTL_DSE_DISABLE             (0 << 3)
> +PAD_CTL_DSE_260ohm              (1 << 3)
> +PAD_CTL_DSE_130ohm              (2 << 3)
> +PAD_CTL_DSE_87ohm               (3 << 3)
> +PAD_CTL_DSE_65ohm               (4 << 3)
> +PAD_CTL_DSE_52ohm               (5 << 3)
> +PAD_CTL_DSE_43ohm               (6 << 3)
> +PAD_CTL_DSE_37ohm               (7 << 3)
> +PAD_CTL_SRE_FAST                (1 << 0)
> +PAD_CTL_SRE_SLOW                (0 << 0)
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
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 4/4] mmc: pwrseq-simple: add disable-post-power-on option
From: Rob Herring @ 2016-12-09 18:09 UTC (permalink / raw)
  To: Matt Ranostay
  Cc: Ulf Hansson, devicetree, linux-mmc, Tony Lindgren, Liam Breck
In-Reply-To: <CAJ_EiSQWN0KF1rJDwzOeoi--g1Z0XNYevYzCLG=NngcdgF6VUw@mail.gmail.com>

On Fri, Dec 02, 2016 at 01:06:47AM -0800, Matt Ranostay wrote:
> On Fri, Dec 2, 2016 at 12:28 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > On 2 December 2016 at 08:22, Matt Ranostay <matt@ranostay.consulting> wrote:
> >>
> >>
> >>> On Dec 1, 2016, at 23:13, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >>>
> >>>> On 2 December 2016 at 07:17, Matt Ranostay <matt@ranostay.consulting> wrote:
> >>>> Add optional device-post-power-on parameters to disable post_power_on
> >>>> function from being called since this breaks some device.
> >>>>
> >>>> Cc: Tony Lindgren <tony@atomide.com>
> >>>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> >>>> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> >>>> ---
> >>>> Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt | 2 ++
> >>>> drivers/mmc/core/pwrseq_simple.c                            | 7 +++++++
> >>>> 2 files changed, 9 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
> >>>> index bea306d772d1..09fa153f743e 100644
> >>>> --- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
> >>>> +++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
> >>>> @@ -24,6 +24,8 @@ Optional properties:
> >>>>        de-asserting the reset-gpios (if any)
> >>>> - invert-off-state: Invert the power down state for the reset-gpios (if any)
> >>>>        and pwrdn-gpios (if any)
> >>>> +- disable-post-power-on : Avoid post_power_on function from being called since
> >>>> +       this breaks some devices
> >>>
> >>> This is a bit weird. I would like to avoid this, if possible.
> >>>
> >>> Perhaps if you simply describe the sequence in detail for your device
> >>> we can figure out the best option together.
> >>
> >> Yeah I know it is a bit weird but we need to keep that reset pin high for at least some time after pre power on.   So any case it would be another property
> >
> > This went offlist, please resend.
> >
> > Moreover, please try to describe the sequence you need in detail
> > according to your HW spec.
> >
> 
> Yeah oops....
> 
> So basically we need to drive the reset and powerdown lines high with
> a 300 milliseconds delay between both...
> can't have the reset line low with post power on (pretty sure but
> would need a delay anyway), and we need both reset + powerdown line
> set low on powerdown.
> 
> So the power down sequence would need to be reversed for this
> application in pwrseq-simple.

This sounds like you need a device specific sequence to me. Otherwise, 
write a language to describe any power control waveforms rather than 
trying to bolt on more and more properties. (Don't really go write a 
language.) 

Rob

^ permalink raw reply

* Re: [PATCH v2 2/2] dt-bindings: Add DT bindings info for FlexRM ring manager
From: Rob Herring @ 2016-12-09 18:01 UTC (permalink / raw)
  To: Anup Patel
  Cc: Jassi Brar, Mark Rutland, Ray Jui, Scott Branden, Pramod KUMAR,
	Rob Rice, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w
In-Reply-To: <1480653536-5551-3-git-send-email-anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

On Fri, Dec 02, 2016 at 10:08:56AM +0530, Anup Patel wrote:
> This patch adds device tree bindings document for the FlexRM
> ring manager found on Broadcom iProc SoCs.
> 
> Reviewed-by: Ray Jui <ray.jui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> ---
>  .../bindings/mailbox/brcm,iproc-flexrm-mbox.txt    | 60 ++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt b/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
> new file mode 100644
> index 0000000..e81f116
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
> @@ -0,0 +1,60 @@

[...]

> +Example:
> +--------
> +crypto_mbox: mbox@67000000 {
> +	compatible = "brcm,flexrm-mbox";
> +	reg = <0x67000000 0x200000>;
> +	msi-parent = <&gic_its 0x7f00>;
> +	#mbox-cells = <3>;
> +};
> +
> +crypto_client {
> +	...
> +	mboxes = <&crypto_mbox 0 0x1 0xffff>,
> +		 <&crypto_mbox 1 0x1 0xffff>,
> +		 <&crypto_mbox 16 0x1 0xffff>,
> +		 <&crypto_mbox 17 0x1 0xffff>,
> +		 <&crypto_mbox 30 0x1 0xffff>,
> +		 <&crypto_mbox 31 0x1 0xffff>;

The FlexRM part looks fine. I still don't understand what this node is. 
Is this a h/w block or just a list of mailboxes? What determines the 
mailbox channel numbers here? I need to see what the complete node looks 
like.

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 net-next v2] dsa:mv88e6xxx: dispose irq mapping for chip->irq
From: Andrew Lunn @ 2016-12-09 18:00 UTC (permalink / raw)
  To: Volodymyr Bendiuga
  Cc: vivien.didelot-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
	volodymyr.bendiuga-Re5JQEeQqe8AvxtiuMwx3w, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4388a669-b425-97e0-346b-6b20f7f47f86-qeDNsGSBLoYwFerOooGFRg@public.gmane.org>

On Wed, Dec 07, 2016 at 05:40:12PM +0100, Volodymyr Bendiuga wrote:
> Yes, most of the users of of_irq_get() do not use irq_dispose_mapping().
> 
> But some of them do (some irq chips), and I believe the correct way
> of doing this is to
> 
> dispose irq mapping, as the description for this function says that
> it unmaps
> 
> the irq, which is mapped by of_irq_parse_and_map(). Not disposing
> irq might not make
> 
> any affect on most drivers, but some, that get EPROBE_DEFER error do
> need to dispose.
> 
> This is what I get when I run the code.
> 
> of_irq_put() could be implemented, and it would be a wrapper for
> irq_dispose_mapping()
> 
> as I can see it. Should I do it this way?

Hi Volodymyr

Yes, i think having of_irq_put() would be good. It gives some symmetry
to the API.

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

^ permalink raw reply

* Re: [PATCH 6/9] dt-bindings: Document rk3399 Gru/Kevin
From: Rob Herring @ 2016-12-09 17:54 UTC (permalink / raw)
  To: Brian Norris
  Cc: Heiko Stuebner, linux-rockchip, linux-kernel, Caesar Wang,
	Doug Anderson, devicetree, Stephen Barber, linux-arm-kernel,
	Chris Zhong
In-Reply-To: <1480645653-36943-7-git-send-email-briannorris@chromium.org>

On Thu, Dec 01, 2016 at 06:27:30PM -0800, Brian Norris wrote:
> Gru is a base dev board for a family of devices, including Kevin. Both
> utilize Rockchip RK3399, and they share much of their design.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
>  Documentation/devicetree/bindings/arm/rockchip.txt | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/rockchip.txt b/Documentation/devicetree/bindings/arm/rockchip.txt
> index cc4ace6397ab..830e13f5890c 100644
> --- a/Documentation/devicetree/bindings/arm/rockchip.txt
> +++ b/Documentation/devicetree/bindings/arm/rockchip.txt
> @@ -99,6 +99,26 @@ Rockchip platforms device tree bindings
>  		     "google,veyron-speedy-rev3", "google,veyron-speedy-rev2",
>  		     "google,veyron-speedy", "google,veyron", "rockchip,rk3288";
>  
> +- Google Gru (dev-board):
> +    Required root node properties:
> +      - compatible = "google,gru-rev15", "google,gru-rev14",
> +		     "google,gru-rev13", "google,gru-rev12",
> +		     "google,gru-rev11", "google,gru-rev10",
> +		     "google,gru-rev9", "google,gru-rev8",
> +		     "google,gru-rev7", "google,gru-rev6",
> +		     "google,gru-rev5", "google,gru-rev4",
> +		     "google,gru-rev3", "google,gru-rev2",
> +		     "google,gru", "rockchip,rk3399";

All of these are supposed to be specified or just one rev at a time?

> +
> +- Google Kevin:
> +    Required root node properties:
> +      - compatible = "google,kevin-rev15", "google,kevin-rev14",
> +		     "google,kevin-rev13", "google,kevin-rev12",
> +		     "google,kevin-rev11", "google,kevin-rev10",
> +		     "google,kevin-rev9", "google,kevin-rev8",
> +		     "google,kevin-rev7", "google,kevin-rev6",
> +		     "google,kevin", "google,gru", "rockchip,rk3399";
> +
>  - mqmaker MiQi:
>      Required root node properties:
>        - compatible = "mqmaker,miqi", "rockchip,rk3288";
> -- 
> 2.8.0.rc3.226.g39d4020
> 

^ 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

* 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

* [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

* [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 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 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 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 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 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


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