Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 1/7] drm: sunxi: Add a basic DRM driver for Allwinner DE2
From: Maxime Ripard @ 2016-11-16 21:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161108153752.a17440e784f2e3993c79ee69@free.fr>

On Tue, Nov 08, 2016 at 03:37:52PM +0100, Jean-Francois Moine wrote:
> On Mon, 7 Nov 2016 23:37:41 +0100
> Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> 
> > Hi,
> > 
> > On Fri, Oct 28, 2016 at 07:34:20PM +0200, Jean-Francois Moine wrote:
> > > On Fri, 28 Oct 2016 00:03:16 +0200
> > > Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> 	[snip]
> > > > > > We've been calling them bus and mod.
> > > > > 
> > > > > I can understand "bus" (which is better than "apb"), but why "mod"?
> > > > 
> > > > Allwinner has been calling the clocks that are supposed to generate
> > > > the external signals (depending on where you were looking) module or
> > > > mod clocks (which is also why we have mod in the clock
> > > > compatibles). The module 1 clocks being used for the audio and the
> > > > module 0 for the rest (SPI, MMC, NAND, display, etc.)
> > > 
> > > I did not find any 'module' in the H3 documentation.
> > > So, is it really a good name?
> > 
> > It's true that they use it less nowadays, but they still do,
> > ie. https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/drivers/clk/sunxi/clk-sun8iw7.c#L513
> 
> There is a 'mod' suffix, but it is used for the bus gates only, not for
> the main clocks.
> 
> > And we have to remain consistent anyway.
> 
> I don't see any consistency in the H3 DT:
> - the bus gates are named "ahb" and apb"

We've been inconsistent in the past, hence why it would be great to
have bus instead.

> - the (main) clocks are named "mmc", "usb0_phy" and "ir"
> There is no "bus" nor "mod".

Look into the other devices, for other SoCs. Pointing out that we've
been bad in the past doesn't mean that we can't improve.

> > > > > > > +
> > > > > > > +- resets: phandle to the reset of the device
> > > > > > > +
> > > > > > > +- ports: phandle's to the LCD ports
> > > > > > 
> > > > > > Please use the OF graph.
> > > > > 
> > > > > These ports are references to the graph of nodes. See
> > > > > 	http://www.kernelhub.org/?msg=911825&p=2
> > > > 
> > > > In an OF-graph, your phandle to the LCD controller would be replaced
> > > > by an output endpoint.
> > > 
> > > This is the DE controller. There is no endpoint link at this level.
> > 
> > The display engine definitely has an endpoint: the TCON.
> 
> Not at all. The video chain is simply
> 	CRTC (TCON) -> connector (HDMI/LCD/DAC/..)
> The DE is an ancillary device which handles the planes.

And what does it do exactly with those planes? It outputs it to the
TCON.

> > > The Device Engine just handles the planes of the LCDs, but, indeed,
> > > the LCDs must know about the DE and the DE must know about the LCDs.
> > > There are 2 ways to realize this knowledge in the DT:
> > > 1) either the DE has one or two phandle's to the LCDs,
> > > 2) or the LCDs have a phandle to the DE.
> > > 
> > > I chose the 1st way, the DE ports pointing to the endpoint of the LCDs
> > > which is part of the video link (OF-graph LCD <-> connector).
> > > It would be possible to have phandles to the LCDs themselves, but this
> > > asks for more code.
> > > 
> > > The second way is also possible, but it also complexifies a bit the
> > > exchanges DE <-> LCD.
> > 
> > I'm still not sure how it would complexify anything, and why you can't
> > use the display graph to model the relation between the display engine
> > and the TCON (and why you want to use a generic property that refers
> > to the of-graph while it really isn't).
> 
> Complexification:
> 1- my solution:
>   At startup time, the DE device is the DRM device.

How do you deal with SoCs with multiple display engines then?

> It has to know the devices entering in the video chains.
>   The CRTCs (LCD/TCON) are found by
> 	ports[i] -> parent
>   The connectors are found by
> 	ports[i] -> endpoint -> remote_endpoint -> parent
> 2- with ports pointing to the LCDs:
>   The CRTCs (LCD/TCON) are simply
> 	ports[i]
>   The connectors are found by
> 	loop on all ports of ports[i]
> 		ports[i][j] -> endpoint -> remote_endpoint -> parent
> 3- with a phandle to the DE in the LCDs:

What do you mean with LCD? Panels? Why would panels have a phandle to
the display engine?

>   The DE cannot be the DRM device because there is no information about
>   the video devices in the DT. Then, the DRM devices are the LCDs.
>   These LCDs must give their indices to the DE. So, the DE must implement
>   some callback function to accept a LCD definition, and there must be
>   a list of DEs in the driver to make the association DE <-> LCD[i]
>   Some more problem may be raised if a user wants to have the same frame
>   buffer on the 2 LCDs of a DE.

I have no idea what you're talking about, sorry.

> Anyway, my solution is already used in the IMX Soc.
> See 'display-subsystem' in arch/arm/boot/dts/imx6q.dtsi for an example.

Pointing out random example in the tree doesn't make a compelling
argument.

> > > > > > > +void de2_disable_vblank(struct drm_device *drm, unsigned crtc)
> > > > > > > +{
> > > > > > > +	struct priv *priv = drm->dev_private;
> > > > > > > +	struct lcd *lcd = priv->lcds[crtc];
> > > > > > > +
> > > > > > > +	tcon_write(lcd->mmio, gint0,
> > > > > > > +			 tcon_read(lcd->mmio, gint0) &
> > > > > > > +					~TCON_GINT0_TCON1_Vb_Int_En);
> > > > > > > +}
> > > > > > > +
> > > > > > > +/* panel functions */
> > > > > > 
> > > > > > Panel functions? In the CRTC driver?
> > > > > 
> > > > > Yes, dumb panel.
> > > > 
> > > > What do you mean by that? Using a Parallel/RGB interface?
> > > 
> > > Sorry, I though this was a well-known name. The 'dump panel' was used
> > > in the documentation of my previous ARM machine as the video frame sent
> > > to the HDMI controller. 'video_frame' is OK for you?
> > 
> > If it's the frame sent to the encoder, then it would be the CRTC by
> > DRM's nomenclature.
> 
> The CRTC is a software entity. The frame buffer is a hardware entity.

Why are you about framebuffer now, this is nowhere in that
discussion. Any way, the framebuffer is also what is put in a plane,
so there's a name collision here, and you'll probably want to change
it.

Judging by the previous discussion, this should really be called
encoder if it encodes the frames on a bus format, or CRTC if it
composes the planes.

> > > > > > > +static const struct {
> > > > > > > +	char chan;
> > > > > > > +	char layer;
> > > > > > > +	char pipe;
> > > > > > > +} plane2layer[DE2_N_PLANES] = {
> > > > > > > +	[DE2_PRIMARY_PLANE] =	{0, 0, 0},
> > > > > > > +	[DE2_CURSOR_PLANE] =	{1, 0, 1},
> > > > > > > +	[DE2_VI_PLANE] =	{0, 1, 0},
> > > > > > > +};
> 	[snip]
> > > > > > 
> > > > > > Comments?
> > > > > 
> > > > > This
> > > > > 	primary plane is channel 0 (VI), layer 0, pipe 0
> > > > > 	cursor plane is channel 1 (UI), layer 0, pipe 1
> > > > > 	overlay plane is channel 0 (VI), layer 1, pipe 0
> > > > > or the full explanation:
> > > > >     Constraints:
> > > > > 	The VI channels can do RGB or YUV, while UI channels can do RGB
> > > > > 	only.
> > > > > 	The LCD 0 has 1 VI channel and 4 UI channels, while
> > > > > 	LCD 1 has only 1 VI channel and 1 UI channel.
> > > > > 	The cursor must go to a channel bigger than the primary channel,
> > > > > 	otherwise it is not transparent.
> > > > >     First try:
> > > > > 	Letting the primary plane (usually RGB) in the 2nd channel (UI),
> > > > > 	as this is done in the legacy driver, asks for the cursor to go
> > > > > 	to the next channel (UI), but this one does not exist in LCD1.
> > > > >     Retained layout:
> > > > > 	So, we must use only 2 channels for the same behaviour on LCD0
> > > > > 	(H3) and LCD1 (A83T)
> > > > > 	The retained combination is:
> > > > > 		- primary plane in the first channel (VI),
> > > > > 		- cursor plane inthe 2nd channel (UI), and
> > > > > 		- overlay plane in the 1st channel (VI).
> > > > > 
> > > > > 	Note that there could be 3 overlay planes (a channel has 4
> > > > > 	layers), but I am not sure that the A83T or the H3 could
> > > > > 	support 3 simultaneous video streams...
> > > > 
> > > > Do you know if the pipe works in the old display engine?
> > > > 
> > > > Especially about the two-steps composition that wouldn't allow you to
> > > > have alpha on all the planes?
> > > > 
> > > > If it is similar, I think hardcoding the pipe number is pretty bad,
> > > > because that would restrict the combination of planes and formats,
> > > > while some other might have worked.
> > > 
> > > From what I understood about the DE2, the pipes just define the priority
> > > of the overlay channels (one pipe for one channel).
> > > With the cursor constraint, there must be at least 2 channels in
> > > order (primary, cursor). Then, with these 2 channels/pipes, there can be
> > > 6 so-called overlay planes (3 RGB/YUV and 3 RGB only).
> > > Enabling the pipes 2 and 3 (LCD 0 only) would offer 8 more planes, but
> > > RGB only. Then, it might be useful to have dynamic pipes.
> > 
> > That's very valuable (and definitely should go into a comment),
> > thanks!
> > 
> > I still believe that's it should be into a (simple at first)
> > atomic_check. That would be easier to extend and quite easy to
> > document and get simply by looking at the code.
> 
> Sorry for I don't understand what you mean.

You can check all the constraints you exposed above in atomic_check
instead of hardcoding it.

> > > > > > > +static int __init de2_drm_init(void)
> > > > > > > +{
> > > > > > > +	int ret;
> > > > > > > +
> > > > > > > +/* uncomment to activate the drm traces at startup time */
> > > > > > > +/*	drm_debug = DRM_UT_CORE | DRM_UT_DRIVER | DRM_UT_KMS |
> > > > > > > +			DRM_UT_PRIME | DRM_UT_ATOMIC; */
> > > > > > 
> > > > > > That's useless.
> > > > > 
> > > > > Right, but it seems that some people don't know how to debug a DRM
> > > > > driver. This is only a reminder.
> > > > > 
> > > > > > > +	DRM_DEBUG_DRIVER("\n");
> > > > > > > +
> > > > > > > +	ret = platform_driver_register(&de2_lcd_platform_driver);
> > > > > > > +	if (ret < 0)
> > > > > > > +		return ret;
> > > > > > > +
> > > > > > > +	ret = platform_driver_register(&de2_drm_platform_driver);
> > > > > > > +	if (ret < 0)
> > > > > > > +		platform_driver_unregister(&de2_lcd_platform_driver);
> > > > > > > +
> > > > > > > +	return ret;
> > > > > > > +}
> > > > > > 
> > > > > > And that really shouldn't be done that way.
> > > > > 
> > > > > May you explain?
> > > > 
> > > > This goes against the whole idea of the device and driver
> > > > model. Drivers should only register themselves, device should be
> > > > created by buses (or by using some external components if the bus
> > > > can't: DT, ACPI, etc.). If there's a match, you get probed.
> > > > 
> > > > A driver that creates its own device just to probe itself violates
> > > > that.
> > > 
> > > In this function (module init), there is no driver yet.
> > > The module contains 2 drivers: the DE (planes) and the LCD (CRTC),
> > > and there is no macro to handle such modules.
> > 
> > Ah, yes, my bad. I thought you were registering a device and a
> > driver. Still this is a very unusual pattern. Why do you need to split
> > the two? Can't you just merge them?
> 
> The DE and the LCDs are different devices on different drivers.
> A DE must be only one device because it has to handle concurent
> accesses from its 2 LCDs. Then 2 drivers.

If it's different drivers, why are they in the same module?

> But only one module. Why? Because there cannot be double direction
> calls from one module to an other one, and, in our case, for example,
> - the DRM (DE) device must call vblank functions which are handled in
>   the CRTC (TCON) device, and
> - the CRTC device must call DE initialization functions at startup time.

I'm sorry, but that doesn't make any sense. The crtc device should
take care of the CRTC functions. Your DRM CRTC and encoders can
definitely be shared across different devices, you can have a look at
how we're doing it for sun4i.

We basically have 3 drivers to create most of the display pipeline:
  - One for the DRM device
  - One for the display engine
  - One for the TCON

Once they have all loaded and registered in the component framework,
their bind callback is called, and it's when the DRM entities are
created using exported functions to manipulate what needs to be setup.

It's definitely doable, it just takes some effort.

> > > > > > > +int de2_plane_init(struct drm_device *drm, struct lcd *lcd)
> > > > > > > +{
> > > > > > > +	int ret, possible_crtcs = 1 << lcd->crtc_idx;
> > > > > > > +
> > > > > > > +	ret = de2_one_plane_init(drm, &lcd->planes[DE2_PRIMARY_PLANE],
> > > > > > > +				DRM_PLANE_TYPE_PRIMARY, possible_crtcs,
> > > > > > > +				ui_formats, ARRAY_SIZE(ui_formats));
> > > > > > > +	if (ret >= 0)
> > > > > > > +		ret = de2_one_plane_init(drm, &lcd->planes[DE2_CURSOR_PLANE],
> > > > > > > +				DRM_PLANE_TYPE_CURSOR, possible_crtcs,
> > > > > > > +				ui_formats, ARRAY_SIZE(ui_formats));
> > > > > > 
> > > > > > Nothing looks really special about that cursor plane. Any reasion not
> > > > > > to make it an overlay?
> > > > > 
> > > > > As explained above (channel/layer/pipe plane definitions), the cursor
> > > > > cannot go in a channel lower or equal to the one of the primary plane.
> > > > > Then, it must be known and, so, have an explicit plane.
> > > > 
> > > > If you were to make it a plane, you could use atomic_check to check
> > > > this and make sure this doesn't happen. And you would gain a generic
> > > > plane that can be used for other purposes if needed.
> > > 
> > > The function drm_crtc_init_with_planes() offers a cursor plane for free.
> > > On the other side, having 6 overlay planes is more than the SoCs can
> > > support.
> > 
> > It's not really for free, it costs you a generic plane that could
> > definitely be used for something else and cannot anymore because
> > they've been hardcoded to a cursor.
> > 
> > And having a camera, the VPU or even an application directly output
> > directly into one of these planes seems a much better use of a generic
> > plane than a cursor.
> 
> Looking at the harder case (A83T), there may be 8 planes on 2 channels.
> Using a primary plane and the cursor,
> 	8 planes - primary plane - cursor plane = 6 planes
> 6 planes are available.
> If I count correctly, in your example:
> 	one camera + one VPU + one application = 3 planes
> 3 planes are used.
> So, 3 planes are still available.
> 
> On the other side, removing the cursor would just let one more plane.
> Do we really need this one? In other words, I'd be pleased to know how
> you run 7 applications doing video overlay.

You can use those planes to do composition too, with each application
having one or more plane. Android uses that.

There's no argument to have a cursor plane, really. Even regular
graphic stack like X can use a regular overlay to have its cursor on
it. It doesn't *remove* anything, it just allows to use the plane for
what it was supposed to be used for.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161116/98fa4f0f/attachment.sig>

^ permalink raw reply

* [upstream-release] [PATCH 1/2] drivers: usb: phy: Add qoriq usb 3.0 phy driver support
From: Scott Wood @ 2016-11-16 21:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <DB5PR0401MB1925E935B4E7F5D1D1E521D5F5BE0@DB5PR0401MB1925.eurprd04.prod.outlook.com>

On 11/16/2016 05:33 AM, Sriram Dash wrote:
>> From: Scott Wood
>> On 11/15/2016 06:39 AM, Sriram Dash wrote:
>>>> From: Scott Wood
>>>> On 11/13/2016 11:27 PM, Sriram Dash wrote:
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt
>>>>> b/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt
>>>>> new file mode 100644
>>>>> index 0000000..d934c80
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt
>>>>> @@ -0,0 +1,36 @@
>>>>> +Driver for Freescale USB 3.0 PHY
>>>>> +
>>>>> +Required properties:
>>>>> +
>>>>> +- compatible :	fsl,qoriq-usb3-phy
>>>>
>>>
>>> Hi Scott,
>>>
>>>> This is a very vague compatible.  Are there versioning registers
>>>> within this register block?
>>>>
>>>
>>> There are versioning registers for the phy (1.0 and 1.1). But the
>>> current erratum
>>> A008751 does not require the mentioning of the version numbers. Was
>>> planning to take care of the versioning when there is code diversity
>>> on the basis of the version number.
>>
>> That is not how device tree bindings work.  The describe the hardware, not the
>> driver.
>>
>> That said, is the block version sufficient to tell whether a given chip has this
>> erratum? If so, you don't need a special property for the erratum.  If not, what is
>> different about the PHY that is not described by the versioning?

Can you find out the answer to this?

>> In any case, it would be nice to mention the version register and its offset in the
>> binding, just so that it becomes part of the definition of this compatible string, and
>> if we come out with some QorIQ chip with a
>> USB3 PHY that is totally different and doesn't have that version register, it'll be
>> clear that it needs a different compatible.
>>
> 
> Okay. Will include version number in the next rev for Documentation and dt.

I'm not asking you to put the version number in the dt if it can be read
from a register.  I'm asking you to have the binding describe the
version register, as part of the definition of what "fsl,qoriq-usb3-phy"
means.

>>> The only reason for __raw_writel is to make the code faster.
>>
>> Does that really matter here?
>>
>>> However, shall I use writel(with both barriers and byte swap) instead
>>
>> Yes, if the registers are little-endian on all chips.
>>
> 
> The endianness is not same for all Socs. But for most Socs, it is big-endian.
> Is "iowrite32be" better instead? 

Then the device tree node needs to say what endian it is, and you need
to choose the appropriate accessor at runtime.

But is it really big endian for most or even any SoCs?  This hardware
isn't present on our PPC chips, right (I checked a couple of the most
recent PPC RMs and it shows USB 2.0 only)?  So it'd just be the few ARM
chips that made almost everything big endian, and even there the couple
RMs I checked (ls1021a and ls1043a) have these registers described as
little-endian.

>>> and then make appropriate changes in the value 32'h27672B2A?
>>
>> Not sure what you mean here.
>>
>>> In my knowledge, there are more than 5 errata in pipeline,
>>
>> Then please get all of these errata described in the device tree ASAP (unless their
>> presence can be reliably inferred from the block version, as discussed above).
>>
> 
> Yes. We will push the errata asap.

My point is that the device tree node should be complete when you submit
it.  Don't wait for the implementation of a workaround to advertise the
existence of an erratum in the device tree.

-Scott

^ permalink raw reply

* [PATCH 1/1] ARM: dts: enable GPIO-b for Broadcom NSP
From: Florian Fainelli @ 2016-11-16 20:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479130219-25639-1-git-send-email-yendapally.reddy@broadcom.com>

On 11/14/2016 05:30 AM, Yendapally Reddy Dhananjaya Reddy wrote:
> This enables the GPIO-b support for Broadcom NSP SoC
> 
> Signed-off-by: Yendapally Reddy Dhananjaya Reddy <yendapally.reddy@broadcom.com>

Applied, thanks
-- 
Florian

^ permalink raw reply

* [PATCH devicetree/next] ARM: BCM5301X: Add DT for TP-LINK Archer C9 V1
From: Florian Fainelli @ 2016-11-16 20:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161113101209.22185-1-zajec5@gmail.com>

On 11/13/2016 02:12 AM, Rafa? Mi?ecki wrote:
> From: Rafa? Mi?ecki <rafal@milecki.pl>
> 
> It's BCM4709A0 based device with 16 MiB flash, 128 MiB of RAM and two
> PCIe based on-PCB BCM4360 chipsets.
> 
> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>

Applied, thanks!
-- 
Florian

^ permalink raw reply

* [PATCH v1 3/3] ARM: dts: Add node for Broadcom OTP controller driver
From: Florian Fainelli @ 2016-11-16 20:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477336324-10543-4-git-send-email-jonathan.richardson@broadcom.com>

On 10/24/2016 12:12 PM, Jonathan Richardson wrote:
> From: Jonathan Richardson <jonathar@broadcom.com>
> 
> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> Tested-by: Jonathan Richardson <jonathan.richardson@broadcom.com>
> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
> Signed-off-by: Oza Pawandeep <oza@broadcom.com>
> Signed-off-by: Jonathan Richardson <jonathan.richardson@broadcom.com>

Applied, thanks
-- 
Florian

^ permalink raw reply

* [PATCH v1 4/4] ARM: dts: Enable interrupt support for cygnus crmu gpio driver
From: Florian Fainelli @ 2016-11-16 20:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476817238-1226-5-git-send-email-jonathan.richardson@broadcom.com>

On 10/18/2016 12:00 PM, Jonathan Richardson wrote:
> The M0 processor handles interrupts for the always-on CRMU GPIO
> controller. Setting the CRMU GPIO driver with the mailbox controller as
> the interrupt parent allows the mailbox controller to forward interrupts
> from the M0 to the GPIO driver for processing.
> 
> Reviewed-by: Jonathan Richardson <jonathan.richardson@broadcom.com>
> Tested-by: Jonathan Richardson <jonathan.richardson@broadcom.com>
> Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com>
> Reviewed-by: Shreesha Rajashekar <shreesha.rajashekar@broadcom.com>
> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> Signed-off-by: Jonathan Richardson <jonathan.richardson@broadcom.com>

Applied, thanks
-- 
Florian

^ permalink raw reply

* [PATCH v1 3/4] ARM: dts: Enable Broadcom iProc mailbox controller
From: Florian Fainelli @ 2016-11-16 20:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476817238-1226-4-git-send-email-jonathan.richardson@broadcom.com>

On 10/18/2016 12:00 PM, Jonathan Richardson wrote:
> Reviewed-by: Jonathan Richardson <jonathan.richardson@broadcom.com>
> Reviewed-by: Shreesha Rajashekar <shreesha.rajashekar@broadcom.com>
> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> Tested-by: Jonathan Richardson <jonathan.richardson@broadcom.com>
> Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> Signed-off-by: Jonathan Richardson <jonathan.richardson@broadcom.com>

Applied, thanks
-- 
Florian

^ permalink raw reply

* [PATCH] pinctrl: sunxi: fix theoretical uninitialized variable access
From: Maxime Ripard @ 2016-11-16 20:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161116141841.2030776-1-arnd@arndb.de>

Hi Arnd,

On Wed, Nov 16, 2016 at 03:18:18PM +0100, Arnd Bergmann wrote:
> gcc warns about a  way that it could use an uninitialized variable:
> 
> drivers/pinctrl/sunxi/pinctrl-sunxi.c: In function 'sunxi_pinctrl_init':
> drivers/pinctrl/sunxi/pinctrl-sunxi.c:1191:8: error: 'best_div' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> This cannot really happen except if 'freq' is UINT_MAX and 'clock' is
> zero, and both of these are forbidden. To shut up the warning anyway,
> this changes the logic to initialize the return code to the first
> divider value before looking at the others.
> 
> Fixes: 7c926492d38a ("pinctrl: sunxi: Add support for interrupt debouncing")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Thanks for that patch.

Just out of curiosity, which gcc gives those warnings? I have 6.2 and
it didn't output anything..

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161116/1ea0fcf3/attachment-0001.sig>

^ permalink raw reply

* [PATCH v3 0/3] modversions: Fix CRC mangling under CONFIG_RELOCATABLE=y
From: Ard Biesheuvel @ 2016-11-16 20:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161116192331.2jwpewu33dwji3fa@pengutronix.de>

On 16 November 2016 at 19:23, Uwe Kleine-K?nig
<u.kleine-koenig@pengutronix.de> wrote:
> On Thu, Oct 27, 2016 at 05:27:08PM +0100, Ard Biesheuvel wrote:
>> This series is a followup to the single patch 'modversions: treat symbol
>> CRCs as 32 bit quantities on 64 bit archs', of which two versions have
>> been sent out so far [0][1]
>>
>> As pointed out by Michael, GNU ld behaves a bit differently between arm64
>> and PowerPC64, and where the former gets rid of all runtime relocations
>> related to CRCs, the latter is not as easily convinced.
>>
>> Patch #1 fixes the issue where CRCs are corrupted by the runtime relocation
>> routines for 32-bit PowerPC, for which the original fix was effectively
>> reverted by commit 0e0ed6406e61 ("powerpc/modules: Module CRC relocation fix
>> causes perf issues")
>>
>> Patch #2 adds handling of R_PPC64_ADDR32 relocations against the NULL .dynsym
>> symbol entry to the PPC64 runtime relocation routines, so it is prepared to
>> deal with CRCs being emitted as 32-bit quantities.
>>
>> Patch #3 is the original patch from the v1 and v2 submissions.
>
> Is this related to me seeing
>
> [    2.111424] mvneta: module verification failed: signature and/or required key missing - tainting kernel
> [    2.126061] scsi_mod: no symbol version for _clear_bit
> [    2.131257] scsi_mod: Unknown symbol _clear_bit (err -22)
> [    2.138093] mvneta: no symbol version for _clear_bit
> [    2.143117] mvneta: Unknown symbol _clear_bit (err -22)
> [    2.144135] mvmdio: no symbol version for __gnu_mcount_nc
> [    2.144138] mvmdio: Unknown symbol __gnu_mcount_nc (err -22)
> ...
>
> ? If so, this would be great to mention it in the commit log to make
> people searching for this issue actually find this patch set.
>

No, I don't think it is. My guess would be that it is caused by

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4dd1837d75

^ permalink raw reply

* [PATCH fpga 8/9] fpga socfpga: Use the scatterlist interface
From: Jason Gunthorpe @ 2016-11-16 20:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <alpine.DEB.2.20.1611160939140.3364@atull-VirtualBox2>

On Wed, Nov 16, 2016 at 09:45:23AM -0600, atull wrote:
> > What is the point of this if write_init gets a copy of the buffer -
> > what is that supposed to be?
> 
> Sometimes write_init needs to look at the header of the image.
> You can see that in the socfpga-a10.c (on linux-next/master)

I know what it is for, I'm asking what should it be if we are calling
write_init multiple times.

It feels like the driver needs to indicate the header length it wants
to inspect and the core core needs to make that much of the bitstream
available to write_init() before calling write().

Is that what you were thinking?

> at this stuff, this is coming at a busy time).  My point there
> was that there was code that needed to go into the core so that
> the ICE40 and the cyclone spi driver that are on the mailing
> list won't have to have the same workaround that you were
> adding to the socfpga.c driver.

Sure, that is easy for write() - not clear on write_init sematics?
I will send a revised series.

I'd also like to close on the zynq bitfile verification patch, did you
have any comments on that?

Jason

^ permalink raw reply

* [PATCH fpga 1/9] fpga zynq: Add missing \n to messages
From: Jason Gunthorpe @ 2016-11-16 20:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161116183926.GA14341@live.com>

On Wed, Nov 16, 2016 at 10:39:26AM -0800, Moritz Fischer wrote:
> > As you say, it is crystal clear already, and this is an acceptable commit
> > message.. Please suggest a text if you want to see something
> > different.
> > 
> 
> It still needs a long message. Just do it.

Can I put your reviewed-by on this when I resend the series?

Will you have time to review the other patches this week?

Jason

^ permalink raw reply

* [PATCH 1/3 v4] ARM: dts: rename MSM8660/APQ8060 pmicintc to pm8058
From: Linus Walleij @ 2016-11-16 20:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478508284-10847-1-git-send-email-linus.walleij@linaro.org>

On Mon, Nov 7, 2016 at 9:44 AM, Linus Walleij <linus.walleij@linaro.org> wrote:

> The name "pmicintc" is ambiguous: there is a second power
> management IC named PM8901 on these systems, and it is also
> an interrupt controller. To make things clear, just name the
> node alias "pm8058", this in unambigous and has all information
> we need.
>
> Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v4:
> - Add Bjorn's ACK.
> - Follow version numbering of the primary patch.

Are these three patches getting applied?

I don't see them in -next or anything...

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH 1/1] drivers: dma-contiguous: Ensure cma reserve region never cross the low/high mem boundary
From: Laura Abbott @ 2016-11-16 20:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479305975-21670-1-git-send-email-jason.hui.liu@nxp.com>

On 11/16/2016 06:19 AM, Jason Liu wrote:
> If the cma reserve region goes through the device-tree method,
> also need ensure the cma reserved region not cross the low/high
> mem boundary. This patch did the similar fix as commit:16195dd
> ("mm: cma: Ensure that reservations never cross the low/high mem boundary")
> 
> Signed-off-by: Jason Liu <jason.hui.liu@nxp.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/base/dma-contiguous.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index e167a1e1..2bc093c 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -244,6 +244,7 @@ static int __init rmem_cma_setup(struct reserved_mem *rmem)
>  {
>  	phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
>  	phys_addr_t mask = align - 1;
> +	phys_addr_t highmem_start;
>  	unsigned long node = rmem->fdt_node;
>  	struct cma *cma;
>  	int err;
> @@ -256,6 +257,32 @@ static int __init rmem_cma_setup(struct reserved_mem *rmem)
>  		pr_err("Reserved memory: incorrect alignment of CMA region\n");
>  		return -EINVAL;
>  	}
> +#ifdef CONFIG_X86
> +	/*
> +	 * high_memory isn't direct mapped memory so retrieving its physical
> +	 * address isn't appropriate.  But it would be useful to check the
> +	 * physical address of the highmem boundary so it's justfiable to get
> +	 * the physical address from it.  On x86 there is a validation check for
> +	 * this case, so the following workaround is needed to avoid it.
> +	 */
> +	highmem_start = __pa_nodebug(high_memory);
> +#else
> +	highmem_start = __pa(high_memory);
> +#endif

The inline #ifdef is not great style, we shouldn't be spreading it around.

> +
> +	/*
> +	 * All pages in the reserved area must come from the same zone.
> +	 * If the reserved region crosses the low/high memory boundary,
> +	 * try to fix it up and then fall back to allocate from the low mem
> +	 */
> +	if (rmem->base < highmem_start &&
> +		(rmem->base + rmem->size) > highmem_start) {
> +		memblock_free(rmem->base, rmem->size);
> +		rmem->base = memblock_alloc_range(rmem->size, align, 0,
> +						highmem_start, MEMBLOCK_NONE);
> +		if (!rmem->base)
> +			return -ENOMEM;
> +	}

Given the alloc happened in the of code, it seems bad form to be
bringing the free and re-alloc here. Perhaps we should be doing the
limiting and checking in the reserved mem code?

If there is no other solution, at the least this deserves a pr_warn
so users know why a reason specified may not be getting requested.

>  
>  	err = cma_init_reserved_mem(rmem->base, rmem->size, 0, &cma);
>  	if (err) {
> 


Thanks,
Laura

^ permalink raw reply

* [PATCH v2] arm/arm64: KVM: VGIC: limit ITARGETSR bits to number of VCPUs
From: Christoffer Dall @ 2016-11-16 19:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161116175716.31578-1-andre.przywara@arm.com>

On Wed, Nov 16, 2016 at 05:57:16PM +0000, Andre Przywara wrote:
> The GICv2 spec says in section 4.3.12 that a "CPU targets field bit that
> corresponds to an unimplemented CPU interface is RAZ/WI."
> Currently we allow the guest to write any value in there and it can
> read that back.
> Mask the written value with the proper CPU mask to be spec compliant.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>

^ permalink raw reply

* [PATCH] pinctrl: sunxi: fix theoretical uninitialized variable access
From: Linus Walleij @ 2016-11-16 19:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161116141841.2030776-1-arnd@arndb.de>

On Wed, Nov 16, 2016 at 3:18 PM, Arnd Bergmann <arnd@arndb.de> wrote:

> gcc warns about a  way that it could use an uninitialized variable:
>
> drivers/pinctrl/sunxi/pinctrl-sunxi.c: In function 'sunxi_pinctrl_init':
> drivers/pinctrl/sunxi/pinctrl-sunxi.c:1191:8: error: 'best_div' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> This cannot really happen except if 'freq' is UINT_MAX and 'clock' is
> zero, and both of these are forbidden. To shut up the warning anyway,
> this changes the logic to initialize the return code to the first
> divider value before looking at the others.
>
> Fixes: 7c926492d38a ("pinctrl: sunxi: Add support for interrupt debouncing")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Patch applied.

Yours,
Linus Walleij

^ permalink raw reply

* [GIT PULL] Allwinner late DT changes for 4.10
From: Linus Walleij @ 2016-11-16 19:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161115210034.mwqnsggmmbzoav77@lukather>

On Tue, Nov 15, 2016 at 10:00 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:

> Here is a pull request that should be merged after the pinctrl PR,
> hence probably in your late PR.
>
> This is just a mechanic conversion to the generic pinconf bindings and
> removal of the useless pinmux properties we had.

This pull request:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

And much encouraged as an awesome cleanup.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH] ARM: dts: imx: ventana: add LTC3676 PMIC support
From: Tim Harvey @ 2016-11-16 19:40 UTC (permalink / raw)
  To: linux-arm-kernel

All of the Gateworks Ventana boards based on the IMX6 SoC except for the
GW54xx use the LTC3676 PMIC. Add a device-tree node with interrupt support
for this PMIC. Additionally remove the simple-bus notation in the regulator
nodes and any fixed regulators that are provided by the PMIC.

Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
 arch/arm/boot/dts/imx6qdl-gw51xx.dtsi | 132 ++++++++++++++++++++-------
 arch/arm/boot/dts/imx6qdl-gw52xx.dtsi | 165 ++++++++++++++++++++++++----------
 arch/arm/boot/dts/imx6qdl-gw53xx.dtsi | 163 +++++++++++++++++++++++----------
 arch/arm/boot/dts/imx6qdl-gw551x.dtsi | 132 +++++++++++++++++++++------
 arch/arm/boot/dts/imx6qdl-gw552x.dtsi | 130 +++++++++++++++++++++------
 arch/arm/boot/dts/imx6qdl-gw553x.dtsi |  98 ++++++++++++++++++--
 6 files changed, 630 insertions(+), 190 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
index afec2c7..341fef1 100644
--- a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
@@ -56,38 +56,29 @@
 		status = "okay";
 	};
 
-	regulators {
-		compatible = "simple-bus";
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		reg_3p3v: regulator at 0 {
-			compatible = "regulator-fixed";
-			reg = <0>;
-			regulator-name = "3P3V";
-			regulator-min-microvolt = <3300000>;
-			regulator-max-microvolt = <3300000>;
-			regulator-always-on;
-		};
+	reg_3p3v: regulator-3p3v {
+		compatible = "regulator-fixed";
+		regulator-name = "3P3V";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-always-on;
+	};
 
-		reg_5p0v: regulator at 1 {
-			compatible = "regulator-fixed";
-			reg = <1>;
-			regulator-name = "5P0V";
-			regulator-min-microvolt = <5000000>;
-			regulator-max-microvolt = <5000000>;
-			regulator-always-on;
-		};
+	reg_5p0v: regulator-5p0v {
+		compatible = "regulator-fixed";
+		regulator-name = "5P0V";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-always-on;
+	};
 
-		reg_usb_otg_vbus: regulator at 2 {
-			compatible = "regulator-fixed";
-			reg = <2>;
-			regulator-name = "usb_otg_vbus";
-			regulator-min-microvolt = <5000000>;
-			regulator-max-microvolt = <5000000>;
-			gpio = <&gpio3 22 GPIO_ACTIVE_HIGH>;
-			enable-active-high;
-		};
+	reg_usb_otg_vbus: regulator-usb-otg-vbus {
+		compatible = "regulator-fixed";
+		regulator-name = "usb_otg_vbus";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		gpio = <&gpio3 22 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
 	};
 };
 
@@ -158,6 +149,81 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_i2c2>;
 	status = "okay";
+
+	pmic: ltc3676 at 3c {
+		compatible = "lltc,ltc3676";
+		reg = <0x3c>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_pmic>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
+
+		regulators {
+			/* VDD_SOC (1+R1/R2 = 1.635) */
+			reg_vdd_soc: sw1 {
+				regulator-name = "vddsoc";
+				regulator-min-microvolt = <674400>;
+				regulator-max-microvolt = <1308000>;
+				lltc,fb-voltage-divider = <127000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_1P8 (1+R1/R2 = 2.505): GPS/VideoIn/ENET-PHY */
+			reg_1p8v: sw2 {
+				regulator-name = "vdd1p8";
+				regulator-min-microvolt = <1033310>;
+				regulator-max-microvolt = <2004000>;
+				lltc,fb-voltage-divider = <301000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_ARM (1+R1/R2 = 1.635) */
+			reg_vdd_arm: sw3 {
+				regulator-name = "vddarm";
+				regulator-min-microvolt = <674400>;
+				regulator-max-microvolt = <1308000>;
+				lltc,fb-voltage-divider = <127000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_DDR (1+R1/R2 = 2.105) */
+			reg_vdd_ddr: sw4 {
+				regulator-name = "vddddr";
+				regulator-min-microvolt = <868310>;
+				regulator-max-microvolt = <1684000>;
+				lltc,fb-voltage-divider = <221000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_2P5 (1+R1/R2 = 3.435): PCIe/ENET-PHY */
+			reg_2p5v: ldo2 {
+				regulator-name = "vdd2p5";
+				regulator-min-microvolt = <2490375>;
+				regulator-max-microvolt = <2490375>;
+				lltc,fb-voltage-divider = <487000 200000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_HIGH (1+R1/R2 = 4.17) */
+			reg_3p0v: ldo4 {
+				regulator-name = "vdd3p0";
+				regulator-min-microvolt = <3023250>;
+				regulator-max-microvolt = <3023250>;
+				lltc,fb-voltage-divider = <634000 200000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+		};
+	};
 };
 
 &i2c3 {
@@ -312,6 +378,12 @@
 			>;
 		};
 
+		pinctrl_pmic: pmicgrp {
+			fsl,pins = <
+				MX6QDL_PAD_GPIO_8__GPIO1_IO08		0x0001b0b0 /* PMIC_IRQ# */
+			>;
+		};
+
 		pinctrl_pps: ppsgrp {
 			fsl,pins = <
 				MX6QDL_PAD_ENET_RXD1__GPIO1_IO26	0x1b0b1
diff --git a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
index 54aca3a..5c76317 100644
--- a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
@@ -71,57 +71,37 @@
 		status = "okay";
 	};
 
-	regulators {
-		compatible = "simple-bus";
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		reg_1p0v: regulator at 0 {
-			compatible = "regulator-fixed";
-			reg = <0>;
-			regulator-name = "1P0V";
-			regulator-min-microvolt = <1000000>;
-			regulator-max-microvolt = <1000000>;
-			regulator-always-on;
-		};
-
-		/* remove this fixed regulator once ltc3676__sw2 driver available */
-		reg_1p8v: regulator at 1 {
-			compatible = "regulator-fixed";
-			reg = <1>;
-			regulator-name = "1P8V";
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <1800000>;
-			regulator-always-on;
-		};
+	reg_1p0v: regulator-1p0v {
+		compatible = "regulator-fixed";
+		regulator-name = "1P0V";
+		regulator-min-microvolt = <1000000>;
+		regulator-max-microvolt = <1000000>;
+		regulator-always-on;
+	};
 
-		reg_3p3v: regulator at 2 {
-			compatible = "regulator-fixed";
-			reg = <2>;
-			regulator-name = "3P3V";
-			regulator-min-microvolt = <3300000>;
-			regulator-max-microvolt = <3300000>;
-			regulator-always-on;
-		};
+	reg_3p3v: regulator-3p3v {
+		compatible = "regulator-fixed";
+		regulator-name = "3P3V";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-always-on;
+	};
 
-		reg_5p0v: regulator at 3 {
-			compatible = "regulator-fixed";
-			reg = <3>;
-			regulator-name = "5P0V";
-			regulator-min-microvolt = <5000000>;
-			regulator-max-microvolt = <5000000>;
-			regulator-always-on;
-		};
+	reg_5p0v: regulator-5p0v {
+		compatible = "regulator-fixed";
+		regulator-name = "5P0V";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-always-on;
+	};
 
-		reg_usb_otg_vbus: regulator at 4 {
-			compatible = "regulator-fixed";
-			reg = <4>;
-			regulator-name = "usb_otg_vbus";
-			regulator-min-microvolt = <5000000>;
-			regulator-max-microvolt = <5000000>;
-			gpio = <&gpio3 22 GPIO_ACTIVE_HIGH>;
-			enable-active-high;
-		};
+	reg_usb_otg_vbus: regulator-usb-otg-vbus {
+		compatible = "regulator-fixed";
+		regulator-name = "usb_otg_vbus";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		gpio = <&gpio3 22 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
 	};
 
 	sound {
@@ -233,6 +213,89 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_i2c2>;
 	status = "okay";
+
+	pmic: ltc3676 at 3c {
+		compatible = "lltc,ltc3676";
+		reg = <0x3c>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_pmic>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
+
+		regulators {
+			/* VDD_SOC (1+R1/R2 = 1.635) */
+			reg_vdd_soc: sw1 {
+				regulator-name = "vddsoc";
+				regulator-min-microvolt = <674400>;
+				regulator-max-microvolt = <1308000>;
+				lltc,fb-voltage-divider = <127000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_1P8 (1+R1/R2 = 2.505): GPS/VideoIn/ENET-PHY */
+			reg_1p8v: sw2 {
+                                regulator-name = "vdd1p8";
+				regulator-min-microvolt = <1033310>;
+				regulator-max-microvolt = <2004000>;
+				lltc,fb-voltage-divider = <301000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_ARM (1+R1/R2 = 1.635) */
+			reg_vdd_arm: sw3 {
+				regulator-name = "vddarm";
+				regulator-min-microvolt = <674400>;
+				regulator-max-microvolt = <1308000>;
+				lltc,fb-voltage-divider = <127000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_DDR (1+R1/R2 = 2.105) */
+			reg_vdd_ddr: sw4 {
+				regulator-name = "vddddr";
+				regulator-min-microvolt = <868310>;
+				regulator-max-microvolt = <1684000>;
+				lltc,fb-voltage-divider = <221000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_2P5 (1+R1/R2 = 3.435): PCIe/ENET-PHY */
+			reg_2p5v: ldo2 {
+				regulator-name = "vdd2p5";
+				regulator-min-microvolt = <2490375>;
+				regulator-max-microvolt = <2490375>;
+				lltc,fb-voltage-divider = <487000 200000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_AUD_1P8: Audio codec */
+			reg_aud_1p8v: ldo3 {
+				regulator-name = "vdd1p8";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-boot-on;
+			};
+
+			/* VDD_HIGH (1+R1/R2 = 4.17) */
+			reg_3p0v: ldo4 {
+				regulator-name = "vdd3p0";
+				regulator-min-microvolt = <3023250>;
+				regulator-max-microvolt = <3023250>;
+				lltc,fb-voltage-divider = <634000 200000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+		};
+	};
 };
 
 &i2c3 {
@@ -467,6 +530,12 @@
 			>;
 		};
 
+		pinctrl_pmic: pmicgrp {
+			fsl,pins = <
+				MX6QDL_PAD_GPIO_8__GPIO1_IO08		0x0001b0b0 /* PMIC_IRQ# */
+			>;
+		};
+
 		pinctrl_pps: ppsgrp {
 			fsl,pins = <
 				MX6QDL_PAD_ENET_RXD1__GPIO1_IO26	0x1b0b1
diff --git a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
index 88e5cb3..8797e7b 100644
--- a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
@@ -72,57 +72,37 @@
 		status = "okay";
 	};
 
-	regulators {
-		compatible = "simple-bus";
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		reg_1p0v: regulator at 0 {
-			compatible = "regulator-fixed";
-			reg = <0>;
-			regulator-name = "1P0V";
-			regulator-min-microvolt = <1000000>;
-			regulator-max-microvolt = <1000000>;
-			regulator-always-on;
-		};
-
-		/* remove when pmic 1p8 regulator available */
-		reg_1p8v: regulator at 1 {
-			compatible = "regulator-fixed";
-			reg = <1>;
-			regulator-name = "1P8V";
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <1800000>;
-			regulator-always-on;
-		};
+	reg_1p0v: regulator-1p0v {
+		compatible = "regulator-fixed";
+		regulator-name = "1P0V";
+		regulator-min-microvolt = <1000000>;
+		regulator-max-microvolt = <1000000>;
+		regulator-always-on;
+	};
 
-		reg_3p3v: regulator at 2 {
-			compatible = "regulator-fixed";
-			reg = <2>;
-			regulator-name = "3P3V";
-			regulator-min-microvolt = <3300000>;
-			regulator-max-microvolt = <3300000>;
-			regulator-always-on;
-		};
+	reg_3p3v: regulator-3p3v {
+		compatible = "regulator-fixed";
+		regulator-name = "3P3V";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-always-on;
+	};
 
-		reg_usb_h1_vbus: regulator at 3 {
-			compatible = "regulator-fixed";
-			reg = <3>;
-			regulator-name = "usb_h1_vbus";
-			regulator-min-microvolt = <5000000>;
-			regulator-max-microvolt = <5000000>;
-			regulator-always-on;
-		};
+	reg_usb_h1_vbus: regulator-usb-h1-vbus {
+		compatible = "regulator-fixed";
+		regulator-name = "usb_h1_vbus";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-always-on;
+	};
 
-		reg_usb_otg_vbus: regulator at 4 {
-			compatible = "regulator-fixed";
-			reg = <4>;
-			regulator-name = "usb_otg_vbus";
-			regulator-min-microvolt = <5000000>;
-			regulator-max-microvolt = <5000000>;
-			gpio = <&gpio3 22 GPIO_ACTIVE_HIGH>;
-			enable-active-high;
-		};
+	reg_usb_otg_vbus: regulator-usb-otg-vbus {
+		compatible = "regulator-fixed";
+		regulator-name = "usb_otg_vbus";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		gpio = <&gpio3 22 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
 	};
 
 	sound {
@@ -226,6 +206,87 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_i2c2>;
 	status = "okay";
+
+	pmic: ltc3676 at 3c {
+		compatible = "lltc,ltc3676";
+		reg = <0x3c>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
+
+		regulators {
+			/* VDD_SOC (1+R1/R2 = 1.635) */
+			reg_vdd_soc: sw1 {
+				regulator-name = "vddsoc";
+				regulator-min-microvolt = <674400>;
+				regulator-max-microvolt = <1308000>;
+				lltc,fb-voltage-divider = <127000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_1P8 (1+R1/R2 = 2.505): GPS/VideoIn/ENET-PHY */
+			reg_1p8v: sw2 {
+                                regulator-name = "vdd1p8";
+				regulator-min-microvolt = <1033310>;
+				regulator-max-microvolt = <2004000>;
+				lltc,fb-voltage-divider = <301000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_ARM (1+R1/R2 = 1.635) */
+			reg_vdd_arm: sw3 {
+				regulator-name = "vddarm";
+				regulator-min-microvolt = <674400>;
+				regulator-max-microvolt = <1308000>;
+				lltc,fb-voltage-divider = <127000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_DDR (1+R1/R2 = 2.105) */
+			reg_vdd_ddr: sw4 {
+				regulator-name = "vddddr";
+				regulator-min-microvolt = <868310>;
+				regulator-max-microvolt = <1684000>;
+				lltc,fb-voltage-divider = <221000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_2P5 (1+R1/R2 = 3.435): PCIe/ENET-PHY */
+			reg_2p5v: ldo2 {
+				regulator-name = "vdd2p5";
+				regulator-min-microvolt = <2490375>;
+				regulator-max-microvolt = <2490375>;
+				lltc,fb-voltage-divider = <487000 200000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_AUD_1P8: Audio codec */
+			reg_aud_1p8v: ldo3 {
+				regulator-name = "vdd1p8a";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-boot-on;
+			};
+
+			/* VDD_HIGH (1+R1/R2 = 4.17) */
+			reg_3p0v: ldo4 {
+				regulator-name = "vdd3p0";
+				regulator-min-microvolt = <3023250>;
+				regulator-max-microvolt = <3023250>;
+				lltc,fb-voltage-divider = <634000 200000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+		};
+	};
 };
 
 &i2c3 {
@@ -456,6 +517,12 @@
 			>;
 		};
 
+		pinctrl_pmic: pmicgrp {
+			fsl,pins = <
+				MX6QDL_PAD_GPIO_8__GPIO1_IO08		0x0001b0b0 /* PMIC_IRQ# */
+			>;
+		};
+
 		pinctrl_pps: ppsgrp {
 			fsl,pins = <
 				MX6QDL_PAD_ENET_RXD1__GPIO1_IO26	0x1b0b1
diff --git a/arch/arm/boot/dts/imx6qdl-gw551x.dtsi b/arch/arm/boot/dts/imx6qdl-gw551x.dtsi
index 4b9fef8..1d44a7a 100644
--- a/arch/arm/boot/dts/imx6qdl-gw551x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw551x.dtsi
@@ -78,34 +78,25 @@
 		reg = <0x10000000 0x20000000>;
 	};
 
-	regulators {
-		compatible = "simple-bus";
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		reg_5p0v: regulator at 0 {
-			compatible = "regulator-fixed";
-			reg = <0>;
-			regulator-name = "5P0V";
-			regulator-min-microvolt = <5000000>;
-			regulator-max-microvolt = <5000000>;
-		};
+	reg_5p0v: regulator-5p0v {
+		compatible = "regulator-fixed";
+		regulator-name = "5P0V";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+	};
 
-		reg_usb_h1_vbus: regulator at 1 {
-			compatible = "regulator-fixed";
-			reg = <1>;
-			regulator-name = "usb_h1_vbus";
-			regulator-min-microvolt = <5000000>;
-			regulator-max-microvolt = <5000000>;
-		};
+	reg_usb_h1_vbus: regulator-usb-h1-vbus {
+		compatible = "regulator-fixed";
+		regulator-name = "usb_h1_vbus";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+	};
 
-		reg_usb_otg_vbus: regulator at 2 {
-			compatible = "regulator-fixed";
-			reg = <2>;
-			regulator-name = "usb_otg_vbus";
-			regulator-min-microvolt = <5000000>;
-			regulator-max-microvolt = <5000000>;
-		};
+	reg_usb_otg_vbus: regulator-usb-otg-vbus {
+		compatible = "regulator-fixed";
+		regulator-name = "usb_otg_vbus";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
 	};
 };
 
@@ -174,6 +165,89 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_i2c2>;
 	status = "okay";
+
+	pmic: ltc3676 at 3c {
+		compatible = "lltc,ltc3676";
+		reg = <0x3c>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_pmic>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
+
+		regulators {
+			/* VDD_SOC (1+R1/R2 = 1.635) */
+			reg_vdd_soc: sw1 {
+				regulator-name = "vddsoc";
+				regulator-min-microvolt = <674400>;
+				regulator-max-microvolt = <1308000>;
+				lltc,fb-voltage-divider = <127000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_DDR (1+R1/R2 = 2.105) */
+			reg_vdd_ddr: sw2 {
+				regulator-name = "vddddr";
+				regulator-min-microvolt = <868310>;
+				regulator-max-microvolt = <1684000>;
+				lltc,fb-voltage-divider = <221000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_ARM (1+R1/R2 = 1.635) */
+			reg_vdd_arm: sw3 {
+				regulator-name = "vddarm";
+				regulator-min-microvolt = <674400>;
+				regulator-max-microvolt = <1308000>;
+				lltc,fb-voltage-divider = <127000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_3P3 (1+R1/R2 = 1.281) */
+			reg_3p3: sw4 {
+				regulator-name = "vdd3p3";
+				regulator-min-microvolt = <1880000>;
+				regulator-max-microvolt = <3647000>;
+				lltc,fb-voltage-divider = <200000 56200>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_1P8a (1+R1/R2 = 2.505): HDMI In core */
+			reg_1p8a: ldo2 {
+				regulator-name = "vdd1p8a";
+				regulator-min-microvolt = <1816125>;
+				regulator-max-microvolt = <1816125>;
+				lltc,fb-voltage-divider = <301000 200000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_1P8b: HDMI In analog */
+			reg_1p8b: ldo3 {
+				regulator-name = "vdd1p8b";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-boot-on;
+			};
+
+			/* VDD_HIGH (1+R1/R2 = 4.17) */
+			reg_3p0: ldo4 {
+				regulator-name = "vdd3p0";
+				regulator-min-microvolt = <3023250>;
+				regulator-max-microvolt = <3023250>;
+				lltc,fb-voltage-divider = <634000 200000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+		};
+	};
 };
 
 &i2c3 {
@@ -308,6 +382,12 @@
 			>;
 		};
 
+		pinctrl_pmic: pmicgrp {
+			fsl,pins = <
+				MX6QDL_PAD_GPIO_8__GPIO1_IO08		0x0001b0b0 /* PMIC_IRQ# */
+			>;
+		};
+
 		pinctrl_pwm2: pwm2grp {
 			fsl,pins = <
 				MX6QDL_PAD_SD1_DAT2__PWM2_OUT		0x1b0b1
diff --git a/arch/arm/boot/dts/imx6qdl-gw552x.dtsi b/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
index ee83161..e22da6f 100644
--- a/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
@@ -55,37 +55,28 @@
 		reg = <0x10000000 0x20000000>;
 	};
 
-	regulators {
-		compatible = "simple-bus";
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		reg_1p0v: regulator at 0 {
-			compatible = "regulator-fixed";
-			reg = <0>;
-			regulator-name = "1P0V";
-			regulator-min-microvolt = <1000000>;
-			regulator-max-microvolt = <1000000>;
-			regulator-always-on;
-		};
+	reg_1p0v: regulator-1p0v {
+		compatible = "regulator-fixed";
+		regulator-name = "1P0V";
+		regulator-min-microvolt = <1000000>;
+		regulator-max-microvolt = <1000000>;
+		regulator-always-on;
+	};
 
-		reg_3p3v: regulator at 2 {
-			compatible = "regulator-fixed";
-			reg = <2>;
-			regulator-name = "3P3V";
-			regulator-min-microvolt = <3300000>;
-			regulator-max-microvolt = <3300000>;
-			regulator-always-on;
-		};
+	reg_3p3v: regulator-3p3v {
+		compatible = "regulator-fixed";
+		regulator-name = "3P3V";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-always-on;
+	};
 
-		reg_5p0v: regulator at 3 {
-			compatible = "regulator-fixed";
-			reg = <3>;
-			regulator-name = "5P0V";
-			regulator-min-microvolt = <5000000>;
-			regulator-max-microvolt = <5000000>;
-			regulator-always-on;
-		};
+	reg_5p0v: regulator-5p0v {
+		compatible = "regulator-fixed";
+		regulator-name = "5P0V";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-always-on;
 	};
 };
 
@@ -148,6 +139,81 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_i2c2>;
 	status = "okay";
+
+	pmic: ltc3676 at 3c {
+		compatible = "lltc,ltc3676";
+		reg = <0x3c>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_pmic>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
+
+		regulators {
+			/* VDD_SOC (1+R1/R2 = 1.635) */
+			reg_vdd_soc: sw1 {
+				regulator-name = "vddsoc";
+				regulator-min-microvolt = <674400>;
+				regulator-max-microvolt = <1308000>;
+				lltc,fb-voltage-divider = <127000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_1P8 (1+R1/R2 = 2.505): ENET-PHY */
+			reg_1p8v: sw2 {
+                                regulator-name = "vdd1p8";
+				regulator-min-microvolt = <1033310>;
+				regulator-max-microvolt = <2004000>;
+				lltc,fb-voltage-divider = <301000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_ARM (1+R1/R2 = 1.635) */
+			reg_vdd_arm: sw3 {
+				regulator-name = "vddarm";
+				regulator-min-microvolt = <674400>;
+				regulator-max-microvolt = <1308000>;
+				lltc,fb-voltage-divider = <127000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_DDR (1+R1/R2 = 2.105) */
+			reg_vdd_ddr: sw4 {
+				regulator-name = "vddddr";
+				regulator-min-microvolt = <868310>;
+				regulator-max-microvolt = <1684000>;
+				lltc,fb-voltage-divider = <221000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_2P5 (1+R1/R2 = 3.435): PCIe/ENET-PHY */
+			reg_2p5v: ldo2 {
+				regulator-name = "vdd2p5";
+				regulator-min-microvolt = <2490375>;
+				regulator-max-microvolt = <2490375>;
+				lltc,fb-voltage-divider = <487000 200000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_HIGH (1+R1/R2 = 4.17) */
+			reg_3p0v: ldo4 {
+				regulator-name = "vdd3p0";
+				regulator-min-microvolt = <3023250>;
+				regulator-max-microvolt = <3023250>;
+				lltc,fb-voltage-divider = <634000 200000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+		};
+	};
 };
 
 &i2c3 {
@@ -260,6 +326,12 @@
 			>;
 		};
 
+		pinctrl_pmic: pmicgrp {
+			fsl,pins = <
+				MX6QDL_PAD_GPIO_8__GPIO1_IO08		0x0001b0b0 /* PMIC_IRQ# */
+			>;
+		};
+
 		pinctrl_pwm2: pwm2grp {
 			fsl,pins = <
 				MX6QDL_PAD_SD1_DAT2__PWM2_OUT		0x1b0b1
diff --git a/arch/arm/boot/dts/imx6qdl-gw553x.dtsi b/arch/arm/boot/dts/imx6qdl-gw553x.dtsi
index 86cec05..1528f98 100644
--- a/arch/arm/boot/dts/imx6qdl-gw553x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw553x.dtsi
@@ -92,14 +92,6 @@
 		status = "okay";
 	};
 
-	reg_3p3v: regulator-3p3v {
-		compatible = "regulator-fixed";
-		regulator-name = "3P0V";
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-		regulator-always-on;
-	};
-
 	reg_5p0v: regulator-5p0v {
 		compatible = "regulator-fixed";
 		regulator-name = "5P0V";
@@ -179,6 +171,89 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_i2c2>;
 	status = "okay";
+
+	pmic: ltc3676 at 3c {
+		compatible = "lltc,ltc3676";
+		reg = <0x3c>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_pmic>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
+
+		regulators {
+			/* VDD_SOC (1+R1/R2 = 1.635) */
+			reg_vdd_soc: sw1 {
+				regulator-name = "vddsoc";
+				regulator-min-microvolt = <674400>;
+				regulator-max-microvolt = <1308000>;
+				lltc,fb-voltage-divider = <127000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_DDR (1+R1/R2 = 2.105) */
+			reg_vdd_ddr: sw2 {
+				regulator-name = "vddddr";
+				regulator-min-microvolt = <868310>;
+				regulator-max-microvolt = <1684000>;
+				lltc,fb-voltage-divider = <221000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_ARM (1+R1/R2 = 1.635) */
+			reg_vdd_arm: sw3 {
+				regulator-name = "vddarm";
+				regulator-min-microvolt = <674400>;
+				regulator-max-microvolt = <1308000>;
+				lltc,fb-voltage-divider = <127000 200000>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_3P3 (1+R1/R2 = 1.281) */
+			reg_3p3v: sw4 {
+				regulator-name = "vdd3p3";
+				regulator-min-microvolt = <1880000>;
+				regulator-max-microvolt = <3647000>;
+				lltc,fb-voltage-divider = <200000 56200>;
+				regulator-ramp-delay = <7000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_1P8a (1+R1/R2 = 2.505): Analog Video Decoder */
+			reg_1p8a: ldo2 {
+				regulator-name = "vdd1p8a";
+				regulator-min-microvolt = <1816125>;
+				regulator-max-microvolt = <1816125>;
+				lltc,fb-voltage-divider = <301000 200000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			/* VDD_1P8b: microSD VDD_1P8 */
+			reg_1p8b: ldo3 {
+				regulator-name = "vdd1p8b";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-boot-on;
+			};
+
+			/* VDD_HIGH (1+R1/R2 = 4.17) */
+			reg_3p0v: ldo4 {
+				regulator-name = "vdd3p0";
+				regulator-min-microvolt = <3023250>;
+				regulator-max-microvolt = <3023250>;
+				lltc,fb-voltage-divider = <634000 200000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+		};
+	};
 };
 
 &i2c3 {
@@ -255,7 +330,6 @@
 	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
 	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
 	cd-gpios = <&gpio7 0 GPIO_ACTIVE_LOW>;
-	vmmc-supply = <&reg_3p3v>;
 	status = "okay";
 };
 
@@ -327,6 +401,12 @@
 		>;
 	};
 
+	pinctrl_pmic: pmicgrp {
+		fsl,pins = <
+			MX6QDL_PAD_GPIO_8__GPIO1_IO08		0x0001b0b0 /* PMIC_IRQ# */
+		>;
+	};
+
 	pinctrl_pps: ppsgrp {
 		fsl,pins = <
 			MX6QDL_PAD_ENET_RXD1__GPIO1_IO26	0x1b0b1
-- 
1.9.1

^ permalink raw reply related

* [PATCH] iommu: mtk: add common-clk dependency
From: Stephen Boyd @ 2016-11-16 19:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161116152837.3508723-1-arnd@arndb.de>

On 11/16, Arnd Bergmann wrote:
> After the MT2701 clock driver was added, we get a harmless warning for
> the iommu driver that selects it, when compile-testing without
> COMMON_CLK.
> 
> warning: (MTK_IOMMU_V1) selects COMMON_CLK_MT2701_IMGSYS which has unmet direct dependencies (COMMON_CLK)
> 
> Adding a dependency on COMMON_CLK avoids the warning.
> 
> Fixes: e9862118272a ("clk: mediatek: Add MT2701 clock support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Hm.. why is an iommu driver selecting a clk driver? They should
be using standard clk APIs so it's not like they need it for
build time correctness. Shouldn't we drop the selects instead?
Those look to have been introduced a few kernel versions ago, but
they were selecting options that didn't exist until a few days
ago when I merged the mediatek clk driver. The clk options are
user-visible, so it should be possible to select them in the
configuration phase.

----8<----
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 8ee54d71c7eb..37e204f3d9be 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -352,9 +352,6 @@ config MTK_IOMMU_V1
 	select IOMMU_API
 	select MEMORY
 	select MTK_SMI
-	select COMMON_CLK_MT2701_MMSYS
-	select COMMON_CLK_MT2701_IMGSYS
-	select COMMON_CLK_MT2701_VDECSYS
 	help
 	  Support for the M4U on certain Mediatek SoCs. M4U generation 1 HW is
 	  Multimedia Memory Managememt Unit. This option enables remapping of

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* [GIT PULL] Allwinner clock changes for 4.10
From: Stephen Boyd @ 2016-11-16 19:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161115212615.lm3guceyfmdjnhfp@lukather>

On 11/15, Maxime Ripard wrote:
> Hi Mike, Stephen,
> 
> Please pull the following patches for the next merge window.
> 
> Thanks!
> Maxime
> 
> The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
> 
>   Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
> 
> are available in the git repository at:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/mripard/linux.git sunxi-clk-for-4.10
> 
> for you to fetch changes up to 0f6f9302b819ca352cfd4f42c18ec08d521f9cae:
> 
>   clk: sunxi-ng: sun8i-h3: Set CLK_SET_RATE_PARENT for audio module clocks (2016-11-11 21:47:41 +0100)

Pulled into clk-next, but I get the following grumbles...

ERROR: space prohibited before that ',' (ctx:WxW)
#282: FILE: drivers/clk/sunxi-ng/ccu-sun50i-a64.c:206:
+                                            "pll-cpux" , "pll-cpux" };
                                                        ^

ERROR: space prohibited before that ',' (ctx:WxW)
#289: FILE: drivers/clk/sunxi-ng/ccu-sun50i-a64.c:213:
+                                            "axi" , "pll-periph0" };
                                                   ^

ERROR: space prohibited before that ',' (ctx:WxE)
#325: FILE: drivers/clk/sunxi-ng/ccu-sun50i-a64.c:249:
+                                            "pll-periph0-2x" ,
                                                              ^

ERROR: space prohibited before that ',' (ctx:WxW)
#333: FILE: drivers/clk/sunxi-ng/ccu-sun50i-a64.c:257:
+static const char * const ahb2_parents[] = { "ahb1" , "pll-periph0" };
                                                     ^

drivers/clk/sunxi-ng/ccu-sun50i-a64.c:160:16: warning: symbol 'pll_mipi_clk' was not declared. Should it be static?
drivers/clk/sunxi-ng/ccu-sun50i-a64.c:507:16: warning: symbol 'tcon1_clk' was not declared. Should it be static?

I'll go fix them this time, but please be more careful next time.

---8<----
diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
index 0674515e2bad..e3c084cc6da5 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
@@ -157,7 +157,7 @@ static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK(pll_gpu_clk, "pll-gpu",
  */
 #define SUN50I_A64_PLL_MIPI_REG		0x040
 
-struct ccu_nkm pll_mipi_clk = {
+static struct ccu_nkm pll_mipi_clk = {
 	.enable		= BIT(31),
 	.lock		= BIT(28),
 	.n		= _SUNXI_CCU_MULT(8, 4),
@@ -203,14 +203,14 @@ static SUNXI_CCU_NM_WITH_GATE_LOCK(pll_ddr1_clk, "pll-ddr1",
 				   CLK_SET_RATE_UNGATE);
 
 static const char * const cpux_parents[] = { "osc32k", "osc24M",
-					     "pll-cpux" , "pll-cpux" };
+					     "pll-cpux", "pll-cpux" };
 static SUNXI_CCU_MUX(cpux_clk, "cpux", cpux_parents,
 		     0x050, 16, 2, CLK_SET_RATE_PARENT | CLK_IS_CRITICAL);
 
 static SUNXI_CCU_M(axi_clk, "axi", "cpux", 0x050, 0, 2, 0);
 
 static const char * const ahb1_parents[] = { "osc32k", "osc24M",
-					     "axi" , "pll-periph0" };
+					     "axi", "pll-periph0" };
 static struct ccu_div ahb1_clk = {
 	.div		= _SUNXI_CCU_DIV_FLAGS(4, 2, CLK_DIVIDER_POWER_OF_TWO),
 
@@ -246,7 +246,7 @@ static SUNXI_CCU_DIV_TABLE(apb1_clk, "apb1", "ahb1",
 			   0x054, 8, 2, apb1_div_table, 0);
 
 static const char * const apb2_parents[] = { "osc32k", "osc24M",
-					     "pll-periph0-2x" ,
+					     "pll-periph0-2x",
 					     "pll-periph0-2x" };
 static SUNXI_CCU_MP_WITH_MUX(apb2_clk, "apb2", apb2_parents, 0x058,
 			     0, 5,	/* M */
@@ -254,7 +254,7 @@ static SUNXI_CCU_MP_WITH_MUX(apb2_clk, "apb2", apb2_parents, 0x058,
 			     24, 2,	/* mux */
 			     0);
 
-static const char * const ahb2_parents[] = { "ahb1" , "pll-periph0" };
+static const char * const ahb2_parents[] = { "ahb1", "pll-periph0" };
 static const struct ccu_mux_fixed_prediv ahb2_fixed_predivs[] = {
 	{ .index = 1, .div = 2 },
 };
@@ -504,7 +504,7 @@ static SUNXI_CCU_MUX_TABLE_WITH_GATE(tcon0_clk, "tcon0", tcon0_parents,
 
 static const char * const tcon1_parents[] = { "pll-video0", "pll-video1" };
 static const u8 tcon1_table[] = { 0, 2, };
-struct ccu_div tcon1_clk = {
+static struct ccu_div tcon1_clk = {
 	.enable		= BIT(31),
 	.div		= _SUNXI_CCU_DIV(0, 4),
 	.mux		= _SUNXI_CCU_MUX_TABLE(24, 2, tcon1_table),

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* [PATCH v3 0/3] modversions: Fix CRC mangling under CONFIG_RELOCATABLE=y
From: Uwe Kleine-König @ 2016-11-16 19:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1477585631-18574-1-git-send-email-ard.biesheuvel@linaro.org>

On Thu, Oct 27, 2016 at 05:27:08PM +0100, Ard Biesheuvel wrote:
> This series is a followup to the single patch 'modversions: treat symbol
> CRCs as 32 bit quantities on 64 bit archs', of which two versions have
> been sent out so far [0][1]
> 
> As pointed out by Michael, GNU ld behaves a bit differently between arm64
> and PowerPC64, and where the former gets rid of all runtime relocations
> related to CRCs, the latter is not as easily convinced.
> 
> Patch #1 fixes the issue where CRCs are corrupted by the runtime relocation
> routines for 32-bit PowerPC, for which the original fix was effectively
> reverted by commit 0e0ed6406e61 ("powerpc/modules: Module CRC relocation fix
> causes perf issues")
> 
> Patch #2 adds handling of R_PPC64_ADDR32 relocations against the NULL .dynsym
> symbol entry to the PPC64 runtime relocation routines, so it is prepared to
> deal with CRCs being emitted as 32-bit quantities.
> 
> Patch #3 is the original patch from the v1 and v2 submissions.

Is this related to me seeing 

[    2.111424] mvneta: module verification failed: signature and/or required key missing - tainting kernel
[    2.126061] scsi_mod: no symbol version for _clear_bit
[    2.131257] scsi_mod: Unknown symbol _clear_bit (err -22)
[    2.138093] mvneta: no symbol version for _clear_bit
[    2.143117] mvneta: Unknown symbol _clear_bit (err -22)
[    2.144135] mvmdio: no symbol version for __gnu_mcount_nc
[    2.144138] mvmdio: Unknown symbol __gnu_mcount_nc (err -22)
...

? If so, this would be great to mention it in the commit log to make
people searching for this issue actually find this patch set.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* [GIT PULL] i.MX clock updates for 4.10
From: Stephen Boyd @ 2016-11-16 19:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161115025711.GU3310@dragon>

On 11/15, Shawn Guo wrote:
> The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
> 
>   Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git tags/imx-clk-4.10
> 
> for you to fetch changes up to 73cd5e53caba2425f5b73ad0950544d1168ad27b:
> 
>   clk: imx: clk-imx6ul: add clk support for imx6ull (2016-11-15 08:55:36 +0800)

Thanks. Pulled into clk-next.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* [PATCH] [media] VPU: mediatek: fix dereference of pdev before checking it is null
From: Colin King @ 2016-11-16 19:16 UTC (permalink / raw)
  To: linux-arm-kernel

From: Colin Ian King <colin.king@canonical.com>

pdev is dereferenced using platform_get_drvdata before a check to
see if it is null, hence there could be a potential null pointer
dereference issue. Instead, first check if pdev is null and only then
deference pdev when initializing vpu.

Found with static analysis by CoverityScan, CID 1357797

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/media/platform/mtk-vpu/mtk_vpu.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/mtk-vpu/mtk_vpu.c b/drivers/media/platform/mtk-vpu/mtk_vpu.c
index c9bf58c..41f31b2 100644
--- a/drivers/media/platform/mtk-vpu/mtk_vpu.c
+++ b/drivers/media/platform/mtk-vpu/mtk_vpu.c
@@ -523,9 +523,9 @@ static int load_requested_vpu(struct mtk_vpu *vpu,
 
 int vpu_load_firmware(struct platform_device *pdev)
 {
-	struct mtk_vpu *vpu = platform_get_drvdata(pdev);
+	struct mtk_vpu *vpu;
 	struct device *dev = &pdev->dev;
-	struct vpu_run *run = &vpu->run;
+	struct vpu_run *run;
 	const struct firmware *vpu_fw = NULL;
 	int ret;
 
@@ -533,6 +533,8 @@ int vpu_load_firmware(struct platform_device *pdev)
 		dev_err(dev, "VPU platform device is invalid\n");
 		return -EINVAL;
 	}
+	vpu = platform_get_drvdata(pdev);
+	run = &vpu->run;
 
 	mutex_lock(&vpu->vpu_mutex);
 	if (vpu->fw_loaded) {
-- 
2.10.2

^ permalink raw reply related

* [GIT PULL] Allwinner clock fixes for 4.9
From: Stephen Boyd @ 2016-11-16 19:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161115212332.w5amyhohmauqzpe5@lukather>

On 11/15, Maxime Ripard wrote:
> Hi Mike, Stephen,
> 
> Please pull the following fixes for the 4.9 cycle
> 
> Thanks!
> Maxime
> 
> The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
> 
>   Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
> 
> are available in the git repository at:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/mripard/linux.git tags/sunxi-clk-fixes-for-4.9
> 
> for you to fetch changes up to ac95330b96376550ae7a533d1396272d675adfa2:
> 
>   clk: sunxi: Fix M factor computation for APB1 (2016-11-04 08:49:46 +0100)

Thanks. Pulled into clk-fixes.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* [PATCH v2] input: touchscreen: silead: Add regulator support
From: Hans de Goede @ 2016-11-16 18:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161116175159.GB335@dtor-ws>

HI,

On 16-11-16 18:51, Dmitry Torokhov wrote:
> On Wed, Nov 16, 2016 at 12:55:07PM +0100, Hans de Goede wrote:
>> On some tablets the touchscreen controller is powered by separate
>> regulators, add support for this.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> Acked-by: Rob Herring <robh@kernel.org>
>> ---
>> Changes in v2:
>> -Use devm_regulator_bulk_get() and friends
>> -Use devm_add_action_or_reset() to disable the regulator
>> ---
>>  .../bindings/input/touchscreen/silead_gsl1680.txt  |  2 ++
>>  drivers/input/touchscreen/silead.c                 | 29 ++++++++++++++++++++++
>>  2 files changed, 31 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt b/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt
>> index e844c3f..b726823 100644
>> --- a/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt
>> @@ -22,6 +22,8 @@ Optional properties:
>>  - touchscreen-inverted-y  : See touchscreen.txt
>>  - touchscreen-swapped-x-y : See touchscreen.txt
>>  - silead,max-fingers	  : maximum number of fingers the touchscreen can detect
>> +- vddio-supply		  : regulator phandle for controller VDDIO
>> +- avdd-supply		  : regulator phandle for controller AVDD
>>
>>  Example:
>>
>> diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c
>> index f502c84..404830a 100644
>> --- a/drivers/input/touchscreen/silead.c
>> +++ b/drivers/input/touchscreen/silead.c
>> @@ -29,6 +29,7 @@
>>  #include <linux/input/touchscreen.h>
>>  #include <linux/pm.h>
>>  #include <linux/irq.h>
>> +#include <linux/regulator/consumer.h>
>>
>>  #include <asm/unaligned.h>
>>
>> @@ -73,6 +74,7 @@ struct silead_ts_data {
>>  	struct i2c_client *client;
>>  	struct gpio_desc *gpio_power;
>>  	struct input_dev *input;
>> +	struct regulator_bulk_data regulators[2];
>>  	char fw_name[64];
>>  	struct touchscreen_properties prop;
>>  	u32 max_fingers;
>> @@ -433,6 +435,13 @@ static int silead_ts_set_default_fw_name(struct silead_ts_data *data,
>>  }
>>  #endif
>>
>> +static void silead_disable_regulator(void *arg)
>> +{
>> +	struct silead_ts_data *data = arg;
>> +
>> +	regulator_bulk_disable(ARRAY_SIZE(data->regulators), data->regulators);
>> +}
>> +
>>  static int silead_ts_probe(struct i2c_client *client,
>>  			   const struct i2c_device_id *id)
>>  {
>> @@ -465,6 +474,26 @@ static int silead_ts_probe(struct i2c_client *client,
>>  	if (client->irq <= 0)
>>  		return -ENODEV;
>>
>> +	data->regulators[0].supply = "vddio";
>> +	data->regulators[1].supply = "avdd";
>> +	error = devm_regulator_bulk_get(dev, ARRAY_SIZE(data->regulators),
>> +					data->regulators);
>> +	if (error)
>> +		return error;
>> +
>> +	/*
>> +	 * Enable regulators at probe and disable them at remove, we need
>> +	 * to keep the chip powered otherwise it forgets its firmware.
>> +	 */
>
> Hmm, this burns power though. Why can't we reload firmware on resume (it
> should be already cached)?

We already put the device in low-power mode using the power pin. Of the
20 or so different tablets I've with this touchscreen controller only
2 actually have a separate regulator for the controller, so I do not
believe that powering down the regulator will be a big win, otherwise
all tablets would have had this.

 > Does it take too long?

It is a couple of kB written one 32-bit word at a time over i2c, so
it's not fast.

Regards,

Hans


>
> Thanks.
>
>> +	error = regulator_bulk_enable(ARRAY_SIZE(data->regulators),
>> +				      data->regulators);
>> +	if (error)
>> +		return error;
>> +
>> +	error = devm_add_action_or_reset(dev, silead_disable_regulator, data);
>> +	if (error)
>> +		return error;
>> +
>>  	/* Power GPIO pin */
>>  	data->gpio_power = devm_gpiod_get_optional(dev, "power", GPIOD_OUT_LOW);
>>  	if (IS_ERR(data->gpio_power)) {
>> --
>> 2.9.3
>>
>

^ permalink raw reply

* [PATCH v8 7/7] arm/arm64: vgic: Implement KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO ioctl
From: Christoffer Dall @ 2016-11-16 18:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478258013-6669-8-git-send-email-vijay.kilari@gmail.com>

On Fri, Nov 04, 2016 at 04:43:33PM +0530, vijay.kilari at gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> 
> Userspace requires to store and restore of line_level for
> level triggered interrupts using ioctl KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO.
> 
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> ---
>  arch/arm64/include/uapi/asm/kvm.h   |  6 +++++
>  virt/kvm/arm/vgic/vgic-kvm-device.c | 50 ++++++++++++++++++++++++++++++++++++-
>  virt/kvm/arm/vgic/vgic-mmio-v3.c    | 11 ++++++++
>  virt/kvm/arm/vgic/vgic-mmio.c       | 33 ++++++++++++++++++++++++
>  virt/kvm/arm/vgic/vgic-mmio.h       |  5 ++++
>  virt/kvm/arm/vgic/vgic.h            |  3 +++
>  6 files changed, 107 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 91c7137..4100f8c 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -211,6 +211,12 @@ struct kvm_arch_memory_slot {
>  #define KVM_DEV_ARM_VGIC_GRP_CTRL	4
>  #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
>  #define KVM_DEV_ARM_VGIC_CPU_SYSREGS    6
> +#define KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO 7
> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT	10
> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK \
> +			(0x3fffffULL << KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT)
> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK	0x3ff
> +#define VGIC_LEVEL_INFO_LINE_LEVEL	0
>  
>  #define   KVM_DEV_ARM_VGIC_CTRL_INIT	0
>  
> diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
> index da532d1..0f82a91 100644
> --- a/virt/kvm/arm/vgic/vgic-kvm-device.c
> +++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
> @@ -512,6 +512,25 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
>  						  regid, reg);
>  		break;
>  	}
> +	case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
> +		unsigned int info, intid;
> +
> +		info = (attr->attr & KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK) >>
> +			KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT;
> +		if (info == VGIC_LEVEL_INFO_LINE_LEVEL) {
> +			if (is_write)
> +				tmp32 = *reg;
> +			intid = attr->attr &
> +				KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK;
> +			ret = vgic_v3_line_level_info_uaccess(vcpu, is_write,
> +							      intid, &tmp32);
> +			if (!is_write)
> +				*reg = tmp32;

I think you can avoid the indirection with tmp32 here by just making the
line level interface use an unsigned long.

> +		} else {
> +			ret = -EINVAL;
> +		}
> +		break;
> +	}
>  	default:
>  		ret = -EINVAL;
>  		break;
> @@ -554,6 +573,17 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
>  
>  		return vgic_v3_attr_regs_access(dev, attr, &reg, true);
>  	}
> +	case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
> +		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
> +		u64 reg;
> +		u32 tmp32;
> +
> +		if (get_user(tmp32, uaddr))
> +			return -EFAULT;
> +
> +		reg = tmp32;
> +		return vgic_v3_attr_regs_access(dev, attr, &reg, true);
> +	}
>  	}
>  	return -ENXIO;
>  }
> @@ -589,8 +619,18 @@ static int vgic_v3_get_attr(struct kvm_device *dev,
>  			return ret;
>  		return put_user(reg, uaddr);
>  	}
> -	}
> +	case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
> +		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
> +		u64 reg;
> +		u32 tmp32;
>  
> +		ret = vgic_v3_attr_regs_access(dev, attr, &reg, false);
> +		if (ret)
> +			return ret;
> +		tmp32 = reg;
> +		return put_user(tmp32, uaddr);
> +	}
> +	}
>  	return -ENXIO;
>  }
>  
> @@ -611,11 +651,19 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
>  		return vgic_v3_has_attr_regs(dev, attr);
>  	case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
>  		return 0;
> +	case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
> +		if (((attr->attr & KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK) >>
> +		      KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT) ==
> +		      VGIC_LEVEL_INFO_LINE_LEVEL)
> +			return 0;
> +		break;
> +	}
>  	case KVM_DEV_ARM_VGIC_GRP_CTRL:
>  		switch (attr->attr) {
>  		case KVM_DEV_ARM_VGIC_CTRL_INIT:
>  			return 0;
>  		}
> +		break;

spurious change?

>  	}
>  	return -ENXIO;
>  }
> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> index 519b919..38b481c 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> @@ -807,3 +807,14 @@ int vgic_v3_redist_uaccess(struct kvm_vcpu *vcpu, bool is_write,
>  		return vgic_uaccess(vcpu, &rd_dev, is_write,
>  				    offset, val);
>  }
> +
> +int vgic_v3_line_level_info_uaccess(struct kvm_vcpu *vcpu, bool is_write,
> +				    u32 intid, u32 *val)
> +{
> +	if (is_write)
> +		vgic_write_irq_line_level_info(vcpu, intid, *val);
> +	else
> +		*val = vgic_read_irq_line_level_info(vcpu, intid);
> +
> +	return 0;
> +}
> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
> index 173d6f0..fb018eb 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio.c
> @@ -371,6 +371,39 @@ void vgic_mmio_write_config(struct kvm_vcpu *vcpu,
>  	}
>  }
>  
> +unsigned long vgic_read_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid)
> +{
> +	int i;
> +	unsigned long val = 0;
> +
> +	for (i = 0; i < 32; i++) {
> +		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
> +
> +		if (irq->line_level)
> +			val |= (1U << i);
> +
> +		vgic_put_irq(vcpu->kvm, irq);
> +	}
> +
> +	return val;
> +}
> +
> +void vgic_write_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid,
> +				    const unsigned long val)
> +{
> +	int i;
> +
> +	for_each_set_bit(i, &val, 32) {

I think you misunderstood this part of the API.  Userspace should be
able to both set an asserted and deasserted line level, regardless of
what the value was before.  So you need to loop through all of them and
set the level as nneded.

> +		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
> +
> +		spin_lock(&irq->irq_lock);
> +		irq->line_level = true;

why don't you have to set pending as well and potentially queue the interrupt?

> +		spin_unlock(&irq->irq_lock);
> +
> +		vgic_put_irq(vcpu->kvm, irq);
> +	}
> +}
> +
>  static int match_region(const void *key, const void *elt)
>  {
>  	const unsigned int offset = (unsigned long)key;
> diff --git a/virt/kvm/arm/vgic/vgic-mmio.h b/virt/kvm/arm/vgic/vgic-mmio.h
> index acbf99e..938702c 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio.h
> +++ b/virt/kvm/arm/vgic/vgic-mmio.h
> @@ -181,6 +181,11 @@ int vgic_validate_mmio_region_addr(struct kvm_device *dev,
>  				   const struct vgic_register_region *regions,
>  				   int nr_regions, gpa_t addr);
>  
> +unsigned long vgic_read_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid);
> +
> +void vgic_write_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid,
> +				    const unsigned long val);
> +
>  unsigned int vgic_v2_init_dist_iodev(struct vgic_io_device *dev);
>  
>  unsigned int vgic_v3_init_dist_iodev(struct vgic_io_device *dev);
> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
> index 0e632d0..77d3d84 100644
> --- a/virt/kvm/arm/vgic/vgic.h
> +++ b/virt/kvm/arm/vgic/vgic.h
> @@ -130,6 +130,9 @@ int vgic_v3_cpu_sysregs_uaccess(struct kvm_vcpu *vcpu, bool is_write,
>  			 u64 id, u64 *val);
>  int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
>  				u64 *reg);
> +int vgic_v3_line_level_info_uaccess(struct kvm_vcpu *vcpu, bool is_write,
> +				    u32 intid, u32 *val);
> +
>  #else
>  static inline int vgic_register_its_iodevs(struct kvm *kvm)
>  {
> -- 
> 1.9.1
> 


Thanks,
-Christoffer

^ permalink raw reply


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