* [PATCH] drm: hdlcd: Work properly in big-endian mode
From: Robin Murphy @ 2016-12-07 16:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <548dc9d9-4d21-7f93-cc92-5bc3234f1b8f@arm.com>
On 07/12/16 16:42, Robin Murphy wrote:
> On 07/12/16 15:57, Daniel Vetter wrote:
>> On Wed, Dec 07, 2016 at 03:31:40PM +0000, Robin Murphy wrote:
>>> Under a big-endian kernel, colours on the framebuffer all come out a
>>> delightful shade of wrong, since we fail to take the reversed byte
>>> order into account. Fortunately, the HDLCD has a control bit to make it
>>> automatically byteswap big-endian data; let's use it as appropriate.
>>>
>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>>
>> fourcc codes (and the drm fourcc codes) are supposed to be little-endian.
>> All of them. So either we fix up the drivers and userspace to set that
>> flag correctly (which would mean hdlcd should expose twice as many
>> formats, each one with the little and big endian mode).
>
> AFAICS, SIMPLEFB_FORMATS *is* supposed to be explicitly little-endian
> modes. I see that DRM_FORMAT_BIG_ENDIAN exists, but nothing in-tree ever
> sets it :/
>
> My vague (and probably wrong) assumption was that the HDLCD is still
> expecting little-endian data, but the the CPU is writing framebuffer
> data as host-endian words, hence what the HDLCD thinks is xRGB is
> actually RGBx under a big-endian kernel - It's certainly consistent
> between kernel (fbcon) and userspace (fb-test[1]): white is yellow, blue
> is black, green is red and red is green. I don't know how to test
> "proper" DRM (I've failed to get X to work with the Arch Linux ARM
> distro I have on there at the moment).
>
> Once again I'm somewhat out of my depth here - I just found a thing that
> seemed appropriate and visibly resolved the immediate problem :)
> By comparison, the same use-cases (fbcon and fb-test) under the same
> big-endian kernel do *not* show the same problem with nouveau driving a
> PCIe 7600GT card, which led me to believe it was HDLCD-specific.
Off the back of that, upon closer inspection, nv_crtc_commit() would
appear to already be doing very much the equivalent thing to what I'm
doing in this patch, so now I have no idea whether this is right or
everything's wrong.
Robin.
> [1]:https://github.com/prpplague/fb-test-app
>
>> Or we nuke the big endian flag from drm fourcc. Adding AMD folks who afaik
>> are the only other ones who ever cared about big endian in drm.
>> -Daniel
>>
>>> ---
>>> drivers/gpu/drm/arm/hdlcd_crtc.c | 7 ++++++-
>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
>>> index 28341b32067f..eceb7bed5dd0 100644
>>> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
>>> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
>>> @@ -63,6 +63,7 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
>>> uint32_t pixel_format;
>>> struct simplefb_format *format = NULL;
>>> int i;
>>> + u32 reg;
>>>
>>> pixel_format = crtc->primary->state->fb->pixel_format;
>>>
>>> @@ -76,7 +77,11 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
>>>
>>> /* HDLCD uses 'bytes per pixel', zero means 1 byte */
>>> btpp = (format->bits_per_pixel + 7) / 8;
>>> - hdlcd_write(hdlcd, HDLCD_REG_PIXEL_FORMAT, (btpp - 1) << 3);
>>> + reg = (btpp - 1) << 3;
>>> +#ifdef __BIG_ENDIAN
>>> + reg |= HDLCD_PIXEL_FMT_BIG_ENDIAN;
>>> +#endif
>>> + hdlcd_write(hdlcd, HDLCD_REG_PIXEL_FORMAT, reg);
>>>
>>> /*
>>> * The format of the HDLCD_REG_<color>_SELECT register is:
>>> --
>>> 2.10.2.dirty
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH 8/9] arm64: dts: rockchip: partially describe PWM regulators for Gru
From: Heiko Stuebner @ 2016-12-07 16:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480645653-36943-9-git-send-email-briannorris@chromium.org>
Hi Brian,
Am Donnerstag, 1. Dezember 2016, 18:27:32 CET schrieb Brian Norris:
> We need to add regulators to the CPU nodes, so cpufreq doesn't think it
> can crank up the clock speed without changing the voltage. However, we
> don't yet have the DT bindings to fully describe the Over Voltage
> Protection (OVP) circuits on these boards. Without that description, we
> might end up changing the voltage too much, too fast.
>
> Add the pwm-regulator descriptions and associate the CPU OPPs, but leave
> them disabled.
>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
is there a specific reason for keeping this change separate?
While it is nice for documentation reasons, as it stands now the previous
patch introduces a regression (cpufreq trying to scale without regulators) and
immediately fixes it here.
So if you're ok with it, I'd like to merge this one back into the previous
patch when applying.
Heiko
^ permalink raw reply
* Tearing down DMA transfer setup after DMA client has finished
From: Måns Rullgård @ 2016-12-07 16:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161207164341.GX6408@localhost>
Vinod Koul <vinod.koul@intel.com> writes:
> On Tue, Dec 06, 2016 at 01:14:20PM +0000, M?ns Rullg?rd wrote:
>>
>> That's not going to work very well. Device drivers typically request
>> dma channels in their probe functions or when the device is opened.
>> This means that reserving one of the few channels there will inevitably
>> make some other device fail to operate.
>
> No that doesnt make sense at all, you should get a channel only when you
> want to use it and not in probe!
Tell that to just about every single driver ever written.
--
M?ns Rullg?rd
^ permalink raw reply
* Tearing down DMA transfer setup after DMA client has finished
From: Måns Rullgård @ 2016-12-07 16:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161207164156.GW6408@localhost>
Vinod Koul <vinod.koul@intel.com> writes:
> On Tue, Dec 06, 2016 at 01:42:31PM +0100, Mason wrote:
>> On 06/12/2016 06:12, Vinod Koul wrote:
>>
>> > On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:
>> >
>> >> Is there a way to write a driver within the existing framework?
>> >
>> > I think so, looking back at comments from Russell, I do tend to agree with
>> > that. Is there a specific reason why sbox can't be tied to alloc and free
>> > channels?
>>
>> Here's a recap of the situation.
>>
>> The "SBOX+MBUS" HW is used in several iterations of the tango SoC:
>
> btw is SBOX setup dependent upon the peripheral connected to?
The sbox is basically a crossbar that connects each of a number of input
ports to any of a number of output ports. A few of the inputs and
outputs are dma channels reading or writing to memory while the rest are
peripheral devices. To perform a mem-to-device transfer, you pick a dma
read channel, program the sbox to connect it to the chosen device, and
finally program the dma channel with address and size to transfer.
>> tango3
>> 2 memory channels available
>> 6 devices ("clients"?) may request an MBUS channel
>
> But only 2 can get a channel at any time..
>
>> tango4 (one more channel)
>> 3 memory channels available
>> 7 devices may request an MBUS channel :
>> NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)
>
> Same here
>
> Only thing is users shouldn't hold on to channel and freeup when not in use.
>
>> Notes:
>> The current NFC driver supports only one controller.
>> IDE is mostly obsolete at this point.
>>
>> tango5 (SATA gets own dedicated MBUS channel pair)
>> 3 memory channels available
>> 5 devices may request an MBUS channel :
>> NFC0, NFC1, memcpy, (IDE0, IDE1)
>>
>>
>> If I understand the current DMA driver (written by Mans), client
>> drivers are instructed to use a specific channel in the DT, and
>> the DMA driver muxes access to that channel. The DMA driver
>> manages a per-channel queue of outstanding DMA transfer requests,
>> and a new transfer is started friom within the DMA ISR
>> (modulo the fact that the interrupt does not signal completion
>> of the transfer, as explained else-thread).
>>
>> What you're proposing, Vinod, is to make a channel exclusive
>> to a driver, as long as the driver has not explicitly released
>> the channel, via dma_release_channel(), right?
>
> Precisely, but yes the downside of that is concurrent access are
> limited, but am not sure if driver implements virtual channels and
> allows that..
My driver implements virtual channels. The problem is that the physical
dma channels signal completion slightly too soon, at least with
mem-to-device transfers. Apparently we need to keep the sbox routing
until the peripheral indicates that it has actually received all the
data.
--
M?ns Rullg?rd
^ permalink raw reply
* [PATCH] drm: hdlcd: Work properly in big-endian mode
From: Ville Syrjälä @ 2016-12-07 16:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161207155714.ldmshptddwvhsdop@phenom.ffwll.local>
On Wed, Dec 07, 2016 at 04:57:14PM +0100, Daniel Vetter wrote:
> On Wed, Dec 07, 2016 at 03:31:40PM +0000, Robin Murphy wrote:
> > Under a big-endian kernel, colours on the framebuffer all come out a
> > delightful shade of wrong, since we fail to take the reversed byte
> > order into account. Fortunately, the HDLCD has a control bit to make it
> > automatically byteswap big-endian data; let's use it as appropriate.
> >
> > Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>
> fourcc codes (and the drm fourcc codes) are supposed to be little-endian.
> All of them. So either we fix up the drivers and userspace to set that
> flag correctly (which would mean hdlcd should expose twice as many
> formats, each one with the little and big endian mode).
>
> Or we nuke the big endian flag from drm fourcc. Adding AMD folks who afaik
> are the only other ones who ever cared about big endian in drm.
I don't like the idea of nuking the flag. If the fb endianness is
defined by the CPU, how would userspace even know that it would have
to byteswap the data when feeding it to the device if the device
and CPU don't agree on the endianness?
> -Daniel
>
> > ---
> > drivers/gpu/drm/arm/hdlcd_crtc.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > index 28341b32067f..eceb7bed5dd0 100644
> > --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> > +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > @@ -63,6 +63,7 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
> > uint32_t pixel_format;
> > struct simplefb_format *format = NULL;
> > int i;
> > + u32 reg;
> >
> > pixel_format = crtc->primary->state->fb->pixel_format;
> >
> > @@ -76,7 +77,11 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
> >
> > /* HDLCD uses 'bytes per pixel', zero means 1 byte */
> > btpp = (format->bits_per_pixel + 7) / 8;
> > - hdlcd_write(hdlcd, HDLCD_REG_PIXEL_FORMAT, (btpp - 1) << 3);
> > + reg = (btpp - 1) << 3;
> > +#ifdef __BIG_ENDIAN
> > + reg |= HDLCD_PIXEL_FMT_BIG_ENDIAN;
> > +#endif
> > + hdlcd_write(hdlcd, HDLCD_REG_PIXEL_FORMAT, reg);
> >
> > /*
> > * The format of the HDLCD_REG_<color>_SELECT register is:
> > --
> > 2.10.2.dirty
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrj?l?
Intel OTC
^ permalink raw reply
* Tearing down DMA transfer setup after DMA client has finished
From: Vinod Koul @ 2016-12-07 16:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <yw1xwpfd2o9f.fsf@unicorn.mansr.com>
On Tue, Dec 06, 2016 at 01:14:20PM +0000, M?ns Rullg?rd wrote:
>
> That's not going to work very well. Device drivers typically request
> dma channels in their probe functions or when the device is opened.
> This means that reserving one of the few channels there will inevitably
> make some other device fail to operate.
No that doesnt make sense at all, you should get a channel only when you
want to use it and not in probe!
--
~Vinod
^ permalink raw reply
* [Question] New mmap64 syscall?
From: Dr. Philipp Tomsich @ 2016-12-07 16:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161207163210.GB31779@e104818-lin.cambridge.arm.com>
Catalin,
> On 07 Dec 2016, at 17:32, Catalin Marinas <catalin.marinas@arm.com> wrote:
>
>>> In other words: Why not keep ILP32 simple an ask users that need a 16TB+ offset
>>> to use LP64? It seems much more consistent with the other choices takes so far.
>>
>> If user can switch to lp64, he doesn't need ilp32 at all, right? :)
>> Also, I don't understand how true 64-bit offset in mmap64() would
>> complicate this port.
>
> It's more like the user wanting a quick transition from code that was
> only ever compiled for AArch32 (or other 32-bit architecture) with a
> goal of full LP64 transition on the long run. I have yet to see
> convincing benchmarks showing ILP32 as an advantage over LP64 (of
> course, I hear the argument of reading a pointer a loop is twice as fast
> with a half-size pointer but I don't consider such benchmarks relevant).
Most of the performance advantage in benchmarks comes from a reduction
in the size of data-structures and/or tighter packing of arrays. In other words,
we can make slightly better use of the caches and push the memory subsystem
a little further when running multiple instances of benchmarks.
Most of these advantages should eventually go away, when struct-reorg makes
it way into the compiler. That said, it?s a marginal (but real) improvement for a
subset of SPEC.
In the real world, the importance of ILP32 as an aid to transition legacy code
that is not 64bit clean? and this should drive the ILP32 discussion. That we
get a boost in our SPEC scores is just a nice extra that we get from it ;-)
Regards,
Philipp.
^ permalink raw reply
* [PATCH] drm: hdlcd: Work properly in big-endian mode
From: Robin Murphy @ 2016-12-07 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161207155714.ldmshptddwvhsdop@phenom.ffwll.local>
On 07/12/16 15:57, Daniel Vetter wrote:
> On Wed, Dec 07, 2016 at 03:31:40PM +0000, Robin Murphy wrote:
>> Under a big-endian kernel, colours on the framebuffer all come out a
>> delightful shade of wrong, since we fail to take the reversed byte
>> order into account. Fortunately, the HDLCD has a control bit to make it
>> automatically byteswap big-endian data; let's use it as appropriate.
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>
> fourcc codes (and the drm fourcc codes) are supposed to be little-endian.
> All of them. So either we fix up the drivers and userspace to set that
> flag correctly (which would mean hdlcd should expose twice as many
> formats, each one with the little and big endian mode).
AFAICS, SIMPLEFB_FORMATS *is* supposed to be explicitly little-endian
modes. I see that DRM_FORMAT_BIG_ENDIAN exists, but nothing in-tree ever
sets it :/
My vague (and probably wrong) assumption was that the HDLCD is still
expecting little-endian data, but the the CPU is writing framebuffer
data as host-endian words, hence what the HDLCD thinks is xRGB is
actually RGBx under a big-endian kernel - It's certainly consistent
between kernel (fbcon) and userspace (fb-test[1]): white is yellow, blue
is black, green is red and red is green. I don't know how to test
"proper" DRM (I've failed to get X to work with the Arch Linux ARM
distro I have on there at the moment).
Once again I'm somewhat out of my depth here - I just found a thing that
seemed appropriate and visibly resolved the immediate problem :)
By comparison, the same use-cases (fbcon and fb-test) under the same
big-endian kernel do *not* show the same problem with nouveau driving a
PCIe 7600GT card, which led me to believe it was HDLCD-specific.
Robin.
[1]:https://github.com/prpplague/fb-test-app
> Or we nuke the big endian flag from drm fourcc. Adding AMD folks who afaik
> are the only other ones who ever cared about big endian in drm.
> -Daniel
>
>> ---
>> drivers/gpu/drm/arm/hdlcd_crtc.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
>> index 28341b32067f..eceb7bed5dd0 100644
>> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
>> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
>> @@ -63,6 +63,7 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
>> uint32_t pixel_format;
>> struct simplefb_format *format = NULL;
>> int i;
>> + u32 reg;
>>
>> pixel_format = crtc->primary->state->fb->pixel_format;
>>
>> @@ -76,7 +77,11 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
>>
>> /* HDLCD uses 'bytes per pixel', zero means 1 byte */
>> btpp = (format->bits_per_pixel + 7) / 8;
>> - hdlcd_write(hdlcd, HDLCD_REG_PIXEL_FORMAT, (btpp - 1) << 3);
>> + reg = (btpp - 1) << 3;
>> +#ifdef __BIG_ENDIAN
>> + reg |= HDLCD_PIXEL_FMT_BIG_ENDIAN;
>> +#endif
>> + hdlcd_write(hdlcd, HDLCD_REG_PIXEL_FORMAT, reg);
>>
>> /*
>> * The format of the HDLCD_REG_<color>_SELECT register is:
>> --
>> 2.10.2.dirty
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
^ permalink raw reply
* Tearing down DMA transfer setup after DMA client has finished
From: Vinod Koul @ 2016-12-07 16:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5846B237.8060409@free.fr>
On Tue, Dec 06, 2016 at 01:42:31PM +0100, Mason wrote:
> On 06/12/2016 06:12, Vinod Koul wrote:
>
> > On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:
> >
> >> Is there a way to write a driver within the existing framework?
> >
> > I think so, looking back at comments from Russell, I do tend to agree with
> > that. Is there a specific reason why sbox can't be tied to alloc and free
> > channels?
>
> Here's a recap of the situation.
>
> The "SBOX+MBUS" HW is used in several iterations of the tango SoC:
btw is SBOX setup dependent upon the peripheral connected to?
>
> tango3
> 2 memory channels available
> 6 devices ("clients"?) may request an MBUS channel
But only 2 can get a channel at any time..
>
> tango4 (one more channel)
> 3 memory channels available
> 7 devices may request an MBUS channel :
> NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)
Same here
Only thing is users shouldn't hold on to channel and freeup when not in use.
> Notes:
> The current NFC driver supports only one controller.
> IDE is mostly obsolete at this point.
>
> tango5 (SATA gets own dedicated MBUS channel pair)
> 3 memory channels available
> 5 devices may request an MBUS channel :
> NFC0, NFC1, memcpy, (IDE0, IDE1)
>
>
> If I understand the current DMA driver (written by Mans), client
> drivers are instructed to use a specific channel in the DT, and
> the DMA driver muxes access to that channel. The DMA driver
> manages a per-channel queue of outstanding DMA transfer requests,
> and a new transfer is started friom within the DMA ISR
> (modulo the fact that the interrupt does not signal completion
> of the transfer, as explained else-thread).
>
> What you're proposing, Vinod, is to make a channel exclusive
> to a driver, as long as the driver has not explicitly released
> the channel, via dma_release_channel(), right?
Precisely, but yes the downside of that is concurrent access are limited, but
am not sure if driver implements virtual channels and allows that..
Thanks
--
~Vinod
^ permalink raw reply
* [PATCH v3 2/5] spi: armada-3700: Add support for the FIFO mode
From: Mark Brown @ 2016-12-07 16:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a9b86043-b631-8154-edc8-5abe49e47114@free-electrons.com>
On Wed, Dec 07, 2016 at 05:19:57PM +0100, Romain Perier wrote:
> Le 05/12/2016 ? 14:37, Mark Brown a ?crit :
> > Why? If the hardware supports a more efficient mode of operation it
> > doesn't seem sensible not to use it.
> That's just that our customer want to keep both modes, this is why I decided
> to use the more efficient mode by default and disable it via a module
> parameter. Previously the more efficient mode was always enabled, based on
> the "compatible string" of the DT (the PIO mode was simply useless in this
> case and dead code)
Keep that out of tree, if the user wants to do silly things then let
them pay the cost.
> > > + if (xfer->rx_buf) {
> > > + /* Set read data length */
> > > + spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG,
> > > + a3700_spi->buf_len);
> > > + /* Start READ transfer */
> > > + val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
> > > + val &= ~A3700_SPI_RW_EN;
> > > + val |= A3700_SPI_XFER_START;
> > > + spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
> > > + } else if (xfer->tx_buf) {
> > > + /* Start Write transfer */
> > So this only supports single duplex transfers but there doesn't seem to
> > be anything that enforces this.
> A flag or a capability, typically? I will investigate
SPI_MASTER_HALF_DUPLEX.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161207/af74c861/attachment.sig>
^ permalink raw reply
* [Question] New mmap64 syscall?
From: Catalin Marinas @ 2016-12-07 16:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161207123944.GA11799@yury-N73SV>
On Wed, Dec 07, 2016 at 06:09:44PM +0530, Yury Norov wrote:
> On Wed, Dec 07, 2016 at 12:07:24PM +0100, Dr.Philipp Tomsich wrote:
> > [Resend, as my mail-client had insisted on using the wrong MIME type?]
> >
> > > On 07 Dec 2016, at 11:34, Yury Norov <ynorov@caviumnetworks.com> wrote:
> > >
> > >> If there is a use case for larger than 16TB offsets, we should add
> > >> the call on all architectures, probably using your approach 3. I don't
> > >> think that we should treat it as anything special for arm64 though.
> > >
> > > From this point of view, 16+TB offset is a matter of 16+TB storage,
> > > and it's more than real. The other consideration to add it is that
> > > we have 64-bit support for offsets in syscalls like sys_llseek().
> > > So mmap64() will simply extend this support.
> >
> > I believe the question is rather if the 16TB offset is a real use-case for ILP32.
>
> This is not for ilp32, but for all 32-bit architectures - both native
> and compat. And because the scope is so generic, I think it's the
> strong reason for us to support true 64-bit offset in mmap().
When I mentioned it, I didn't realise that we already use 6 registers
for mmap(). While we can go up to 8 on AArch64/ILP32, I think Arnd has a
point that we don't want this to diverge from other new 32-bit
architectures. I don't really have a strong opinion either way here,
just a remark that AArch64/ILP32 already diverged from _current_ 32-bit
architectures by introducing 64-bit off_t in a 32-bit world. Introducing
an mmap64() at the same time wouldn't look too bad either.
> > This seems to bring the discussion full-circle, as this would indicate that 64bit is the
> > preferred bit-width for all sizes, offsets, etc. throughout all filesystem-related calls
> > (i.e. stat, seek, etc.).
>
> AARCH64/ILP32 (and all new arches) exposes ino_t, off_t, blkcnt_t,
> fsblkcnt_t, fsfilcnt_t and rlim_t as 64-bit types. (Size_t should
> be 32-bit of course, because it's the same lengths as pointer.)
>
> It allows to make syscalls that pass it support 64-bit values, refer
> Documentation/arm64/ilp32.txt for details. Stat and seek are both
> supporting 64-bit types. From this point of view, mmap() is the (only?)
> exception in current ILP32 ABI.
I thought ILP32 will use llseek() which has its own explicit way of
passing a 64-bit offset and the result written back by the kernel. We
wouldn't be able to use lseek() because of the return type.
> > But if that is the case, then we should have gone with 64bit arguments in a single
> > register for our ILP32 definition on AArch64.
>
> There are 2 unrelated matters - the size of types, and the size of
> register. Most of 32-bit architectures has hardware limitation on
> register size (consider aarch32). And it doesn't mean that they are
> forced to stuck with 32-bit off_t etc. This is still opened question
> how to pass 64-bit parameters in aarch64/ilp32 because there we have
> the choice (the reason why it's RFC). If you have new ideas - welcome
> to that discussion. This topic also covers architectures that has to
> pass 64-bit parameters in a pair.
We've discussed this a few times already and the only sane option from
the _kernel_ perspective seemed to be either (a) close to native ABI for
ILP32 (and breaking POSIX) or (b) just a standard 32-bit ABI. The latter
implies splitting 64-bit values in register pairs, especially to avoid a
lot of annotations/wrapping in the generic kernel unistd.h file. IIRC,
we decided to go with option (b), so I don't think it's worth re-opening
that discussion.
> > In other words: Why not keep ILP32 simple an ask users that need a 16TB+ offset
> > to use LP64? It seems much more consistent with the other choices takes so far.
>
> If user can switch to lp64, he doesn't need ilp32 at all, right? :)
> Also, I don't understand how true 64-bit offset in mmap64() would
> complicate this port.
It's more like the user wanting a quick transition from code that was
only ever compiled for AArch32 (or other 32-bit architecture) with a
goal of full LP64 transition on the long run. I have yet to see
convincing benchmarks showing ILP32 as an advantage over LP64 (of
course, I hear the argument of reading a pointer a loop is twice as fast
with a half-size pointer but I don't consider such benchmarks relevant).
--
Catalin
^ permalink raw reply
* [PATCH] i2c: rk3x: keep i2c irq ON in suspend
From: Grygorii Strashko @ 2016-12-07 16:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a6694d7d-6fca-e5ef-7bbd-9956562132a2@rock-chips.com>
On 12/06/2016 09:37 PM, David.Wu wrote:
> Hi Doug,
>
> ? 2016/12/7 0:31, Doug Anderson ??:
>> Hi,
>>
>> On Tue, Dec 6, 2016 at 12:12 AM, David.Wu <david.wu@rock-chips.com>
>> wrote:
>>> Hi Heiko,
>>>
>>> ? 2016/12/5 18:54, Heiko Stuebner ??:
>>>>
>>>> Hi David,
>>>>
>>>> Am Montag, 5. Dezember 2016, 16:02:59 CET schrieb David Wu:
>>>>>
>>>>> During suspend there may still be some i2c access happening.
>>>>> And if we don't keep i2c irq ON, there may be i2c access timeout if
>>>>> i2c is in irq mode of operation.
>>>>
>>>>
>>>> can you describe the issue you're trying to fix a bit more please?
>>>
>>>
>>> Sometimes we could see the i2c timeout errors during suspend/resume,
>>> which
>>> makes the duration of suspend/resume too longer.
>>>
>>> [ 484.171541] CPU4: Booted secondary processor [410fd082]
>>> [ 485.172777] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
>>> [ 486.172760] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
>>> [ 487.172759] rk3x-i2c ff3c0000.i2c: timeout, ipd: 0x10, state: 1
>>> [ 487.172840] cpu cpu4: _set_opp_voltage: failed to set voltage (800000
>>> 800000 800000 mV): -110
>>> [ 487.172874] cpu cpu4: failed to set volt 800000
>>>
>>>>
>>>> I.e. I'd think the i2c-core does suspend i2c-client devices first,
>>>> so that
>>>> these should be able to finish up their ongoing transfers and not start
>>>> any
>>>> new ones instead?
>>>>
>>>> Your irq can still happen slightly after the system started going to
>>>> actually
>>>> sleep, so to me it looks like you just widened the window where irqs
>>>> can
>>>> be
>>>> handled. Especially as your irq could also just simply stem from the
>>>> start
>>>> state, so you cannot even be sure if your transaction actually is
>>>> finished.
>>>
>>>
>>> Okay, you are right. I want to give it a double insurance at first,
>>> but it
>>> may hide the unhappend issue.
>>>
>>>>
>>>> So to me it looks like the i2c-connected device driver should be fixed
>>>> instead?
>>>
>>>
>>> I tell them to fix it in rk808 driver.
>>
>> To me it seems like perhaps cpufreq should not be changing frequencies
>> until it is resumed properly. Presumably if all the ordering is done
>> right then cpufreq should be resumed _after_ the i2c regulator so you
>> should be OK. ...or am I somehow confused about that?
>
> yes?the cpufreq and regulator should start i2c job after they resume
> properly.
>
>>
>> Also note that previous i2c busses I worked with simply returned -EIO
>> in the case where they were called when suspended. See
>> "i2c-exynos5.c" and "i2c-s3c2410.c".
>
> In "i2c-exynos5.c", it seems that using the "i2c->suspended" to protect
> i2c transfer works most of the time. Of course it could prevent the next
> new i2c transfer to start. But in one case, if the current i2c job was
> not finished until the i2c irq was disabled by system suspend, the i2c
> timeout error would also happen, as the current i2c job may have a large
> data to transfer and it lasts from a long time.
And this means you have bug in some of I2C client drivers which do not stop
their activities during suspend properly (most usual case - driver uses work
and this work still active during suspend and can run on one CPU while suspend
runs on another).
At the moment .suspend_noirq() callback is called there should be no active
I2C transactions in general.
>
> So is it necessary to add a mutex lock to wait the current job to be
> finished before the "i2c->suspended" is changed in i2c_suspend_noirq()?
>
You need to catch and fix all driver who will try to access I2C after your
I2C bus driver passes suspend_noirq stage. Smth, like [1], uses i2c_lock_adapter().
[1] https://git.ti.com/android-sdk/kernel-omap/commit/125ef8f7016e7b205886f93862288a45a312b1d8
--
regards,
-grygorii
^ permalink raw reply
* [PATCH v3 2/5] spi: armada-3700: Add support for the FIFO mode
From: Romain Perier @ 2016-12-07 16:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161205133728.sza2pt4tfkywu35k@sirena.org.uk>
Hello,
Le 05/12/2016 ? 14:37, Mark Brown a ?crit :
> On Thu, Dec 01, 2016 at 11:27:16AM +0100, Romain Perier wrote:
>
>> Changes in v3:
>> - Don't enable the fifo mode based on the compatible string, we introduce
>> a module parameter "pio_mode". By default this option is set to zero, so
>> the fifo mode is enabled. Pass pio_mode=1 to the driver enables the PIO
>> mode.
>
> Why? If the hardware supports a more efficient mode of operation it
> doesn't seem sensible not to use it.
That's just that our customer want to keep both modes, this is why I
decided to use the more efficient mode by default and disable it via a
module parameter. Previously the more efficient mode was always enabled,
based on the "compatible string" of the DT (the PIO mode was simply
useless in this case and dead code)
>
>> - int i;
>> + int i, ret = 0;
>
> Coding style - don't combine initialized and non-initalized variables on
> one line.
Ok
>
>> static int a3700_spi_transfer_one(struct spi_master *master,
>> struct spi_device *spi,
>> struct spi_transfer *xfer)
>> {
>> struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
>> - int ret = 0;
>> + int ret;
>>
>> a3700_spi_transfer_setup(spi, xfer);
>>
>> @@ -505,6 +737,151 @@ static int a3700_spi_transfer_one(struct spi_master *master,
>> return ret;
>> }
>
> This appears to be a random unrelated change, should probably be part of
> the initial driver commit.
Good catch
>
>> +static int a3700_spi_fifo_transfer_one(struct spi_master *master,
>> + struct spi_device *spi,
>> + struct spi_transfer *xfer)
>> +{
>> + struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
>> + int ret = 0, timeout = A3700_SPI_TIMEOUT;
>> + unsigned int nbits = 0;
>> + u32 val;
>> +
>> + a3700_spi_transfer_setup(spi, xfer);
>> +
>> + a3700_spi->tx_buf = xfer->tx_buf;
>> + a3700_spi->rx_buf = xfer->rx_buf;
>> + a3700_spi->buf_len = xfer->len;
>> +
>> + /* SPI transfer headers */
>> + a3700_spi_header_set(a3700_spi);
>> +
>> + if (xfer->tx_buf)
>> + nbits = xfer->tx_nbits;
>> + else if (xfer->rx_buf)
>> + nbits = xfer->rx_nbits;
>> +
>> + a3700_spi_pin_mode_set(a3700_spi, nbits);
>> +
>> + if (xfer->rx_buf) {
>> + /* Set read data length */
>> + spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG,
>> + a3700_spi->buf_len);
>> + /* Start READ transfer */
>> + val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
>> + val &= ~A3700_SPI_RW_EN;
>> + val |= A3700_SPI_XFER_START;
>> + spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
>> + } else if (xfer->tx_buf) {
>> + /* Start Write transfer */
>
> So this only supports single duplex transfers but there doesn't seem to
> be anything that enforces this.
>
A flag or a capability, typically? I will investigate
Thanks,
Romain
--
Romain Perier, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH] drm: hdlcd: Work properly in big-endian mode
From: liviu.dudau at arm.com @ 2016-12-07 16:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f484518775c0bf063ae0a6f1e6c11bb404a7acc9.1481124156.git.robin.murphy@arm.com>
On Wed, Dec 07, 2016 at 03:31:40PM +0000, Robin Murphy wrote:
> Under a big-endian kernel, colours on the framebuffer all come out a
> delightful shade of wrong,
So you are saying that wrong is only a 1 bit value?
> since we fail to take the reversed byte
> order into account. Fortunately, the HDLCD has a control bit to make it
> automatically byteswap big-endian data; let's use it as appropriate.
>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Change looks good to me, but as Daniel has mentioned, we should have failed
the modesetting as the buffer we are passed should be in the wrong fourcc format.
Any way I can play with a big-endian filesystem that you can share?
Best regards,
Liviu
> ---
> drivers/gpu/drm/arm/hdlcd_crtc.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
> index 28341b32067f..eceb7bed5dd0 100644
> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -63,6 +63,7 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
> uint32_t pixel_format;
> struct simplefb_format *format = NULL;
> int i;
> + u32 reg;
>
> pixel_format = crtc->primary->state->fb->pixel_format;
>
> @@ -76,7 +77,11 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
>
> /* HDLCD uses 'bytes per pixel', zero means 1 byte */
> btpp = (format->bits_per_pixel + 7) / 8;
> - hdlcd_write(hdlcd, HDLCD_REG_PIXEL_FORMAT, (btpp - 1) << 3);
> + reg = (btpp - 1) << 3;
> +#ifdef __BIG_ENDIAN
> + reg |= HDLCD_PIXEL_FMT_BIG_ENDIAN;
> +#endif
> + hdlcd_write(hdlcd, HDLCD_REG_PIXEL_FORMAT, reg);
>
> /*
> * The format of the HDLCD_REG_<color>_SELECT register is:
> --
> 2.10.2.dirty
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
?\_(?)_/?
^ permalink raw reply
* [PATCH 6/7] arm: Add livepatch to build if CONFIG_LIVEPATCH
From: Abel Vesa @ 2016-12-07 16:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161207150525.GD13288@linux.suse>
On Wed, Dec 07, 2016 at 04:05:25PM +0100, Petr Mladek wrote:
> On Tue 2016-12-06 17:06:06, Abel Vesa wrote:
> > Necessary livepatch file added to makefile.
> >
> > Signed-off-by: Abel Vesa <abelvesa@linux.com>
> > ---
> > arch/arm/kernel/Makefile | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> > index ad325a8..9e70220 100644
> > --- a/arch/arm/kernel/Makefile
> > +++ b/arch/arm/kernel/Makefile
> > @@ -48,6 +48,7 @@ obj-$(CONFIG_HAVE_ARM_TWD) += smp_twd.o
> > obj-$(CONFIG_ARM_ARCH_TIMER) += arch_timer.o
> > obj-$(CONFIG_FUNCTION_TRACER) += entry-ftrace.o
> > obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o insn.o
> > +obj-$(CONFIG_LIVEPATCH) += livepatch.o
>
> It is strange that you add a source file in one patch and make it
> build in a much later patch.
>
> I suggest to restructure the entire patchset a bit. Please, first
> add support for FTRACE_WITH_REGS. It makes sense on its own.
> Then add the livepatch support on top of it.
>
> Otherwise, it is not necessary to send v2 immediately for such
> non-trivial code. There might be more people that would want
> to look at it and it might take days until they find a time.
> It is always better to collect some feedback, think about it
> over night(s). Every question often opens many other questions
> and it usually takes some time until all settles down into
> a good picture again.
>
> Best Regards,
> Petr
You're right, I should send this into two steps. One patchset that
adds FTRACE_WITH_REGS and then a second one that implements the
livepatch and is based on the first one.
Will do that.
Thanks.
^ permalink raw reply
* [PATCH v4 1/4] [media] davinci: vpif_capture: don't lock over s_stream
From: Kevin Hilman @ 2016-12-07 16:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4999781.kd7ueUSsQd@avalon>
Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
> Hi Kevin,
>
> On Tuesday 06 Dec 2016 08:49:38 Kevin Hilman wrote:
>> Laurent Pinchart writes:
>> > On Tuesday 29 Nov 2016 15:57:09 Kevin Hilman wrote:
>> >> Video capture subdevs may be over I2C and may sleep during xfer, so we
>> >> cannot do IRQ-disabled locking when calling the subdev.
>> >>
>> >> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
>> >> ---
>> >> drivers/media/platform/davinci/vpif_capture.c | 3 +++
>> >> 1 file changed, 3 insertions(+)
>> >>
>> >> diff --git a/drivers/media/platform/davinci/vpif_capture.c
>> >> b/drivers/media/platform/davinci/vpif_capture.c index
>> >> 5104cc0ee40e..9f8f41c0f251 100644
>> >> --- a/drivers/media/platform/davinci/vpif_capture.c
>> >> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> >> @@ -193,7 +193,10 @@ static int vpif_start_streaming(struct vb2_queue
>> >> *vq, unsigned int count)
>> >> }
>> >> }
>> >>
>> >> + spin_unlock_irqrestore(&common->irqlock, flags);
>> >> ret = v4l2_subdev_call(ch->sd, video, s_stream, 1);
>> >> + spin_lock_irqsave(&common->irqlock, flags);
>> >
>> > I always get anxious when I see a spinlock being released randomly with an
>> > operation in the middle of a protected section. Looking at the code it
>> > looks like the spinlock is abused here. irqlock should only protect the
>> > dma_queue and should thus only be taken around the following code:
>> >
>> > spin_lock_irqsave(&common->irqlock, flags);
>> > /* Get the next frame from the buffer queue */
>> > common->cur_frm = common->next_frm = list_entry(common->dma_queue.next,
>> > struct vpif_cap_buffer, list);
>> >
>> > /* Remove buffer from the buffer queue */
>> > list_del(&common->cur_frm->list);
>> > spin_unlock_irqrestore(&common->irqlock, flags);
>>
>> Yes, that looks correct. Will update.
>>
>> > The code that is currently protected by the lock in the start and stop
>> > streaming functions should be protected by a mutex instead.
>>
>> I tried taking the mutex here, but lockdep pointed out a deadlock. I
>> may not be fully understanding the V4L2 internals here, but it seems
>> that the ioctl is already taking a mutex, so taking it again in
>> start/stop streaming is a deadlock. Unless you think the locking should
>> be nested here, it seems to me that the mutex isn't needed.
>
> The V4L2 core can lock all ioctls using struct video_device::lock. For buffer-
> related ioctls, it can optionally use a separate lock from struct
> vb2_queue::lock. See v4l2_ioctl_get_lock() in drivers/media/v4l2-core/v4l2-
> ioctl.c.
>
> The vpif-capture driver sets both the video_device and vb2_queue locks to the
> same lock (which would have the same effect as leaving the vb2_queue lock
> NULL). All ioctls are thus serialized. You would only need to handle locking
> in start_streaming and stop_streaming manually if you didn't rely on the core
> serializing the ioctls.
OK, thanks for clarifying how that works.
Kevin
^ permalink raw reply
* [PATCH] drm: hdlcd: Work properly in big-endian mode
From: Daniel Vetter @ 2016-12-07 15:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f484518775c0bf063ae0a6f1e6c11bb404a7acc9.1481124156.git.robin.murphy@arm.com>
On Wed, Dec 07, 2016 at 03:31:40PM +0000, Robin Murphy wrote:
> Under a big-endian kernel, colours on the framebuffer all come out a
> delightful shade of wrong, since we fail to take the reversed byte
> order into account. Fortunately, the HDLCD has a control bit to make it
> automatically byteswap big-endian data; let's use it as appropriate.
>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
fourcc codes (and the drm fourcc codes) are supposed to be little-endian.
All of them. So either we fix up the drivers and userspace to set that
flag correctly (which would mean hdlcd should expose twice as many
formats, each one with the little and big endian mode).
Or we nuke the big endian flag from drm fourcc. Adding AMD folks who afaik
are the only other ones who ever cared about big endian in drm.
-Daniel
> ---
> drivers/gpu/drm/arm/hdlcd_crtc.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
> index 28341b32067f..eceb7bed5dd0 100644
> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -63,6 +63,7 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
> uint32_t pixel_format;
> struct simplefb_format *format = NULL;
> int i;
> + u32 reg;
>
> pixel_format = crtc->primary->state->fb->pixel_format;
>
> @@ -76,7 +77,11 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
>
> /* HDLCD uses 'bytes per pixel', zero means 1 byte */
> btpp = (format->bits_per_pixel + 7) / 8;
> - hdlcd_write(hdlcd, HDLCD_REG_PIXEL_FORMAT, (btpp - 1) << 3);
> + reg = (btpp - 1) << 3;
> +#ifdef __BIG_ENDIAN
> + reg |= HDLCD_PIXEL_FMT_BIG_ENDIAN;
> +#endif
> + hdlcd_write(hdlcd, HDLCD_REG_PIXEL_FORMAT, reg);
>
> /*
> * The format of the HDLCD_REG_<color>_SELECT register is:
> --
> 2.10.2.dirty
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* [PATCH 2/3] crypto: brcm: Add Broadcom SPU driver
From: Rob (William) Rice @ 2016-12-07 15:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206141826.GC24177@leverpostej>
Mark,
Thanks for the comments. Replies below.
Rob
On 12/6/2016 9:18 AM, Mark Rutland wrote:
> On Wed, Nov 30, 2016 at 03:07:32PM -0500, Rob Rice wrote:
>> +static const struct of_device_id bcm_spu_dt_ids[] = {
>> + {
>> + .compatible = "brcm,spum-crypto",
>> + .data = &spum_ns2_types,
>> + },
>> + {
>> + .compatible = "brcm,spum-nsp-crypto",
>> + .data = &spum_nsp_types,
>> + },
>> + {
>> + .compatible = "brcm,spu2-crypto",
>> + .data = &spu2_types,
>> + },
>> + {
>> + .compatible = "brcm,spu2-v2-crypto",
>> + .data = &spu2_v2_types,
>> + },
> These last two weren't in the binding document.
yes, I'll add them.
>
>> + { /* sentinel */ }
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, bcm_spu_dt_ids);
>> +
>> +static int spu_dt_read(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct spu_hw *spu = &iproc_priv.spu;
>> + struct device_node *dn = pdev->dev.of_node;
>> + struct resource *spu_ctrl_regs;
>> + const struct of_device_id *match;
>> + struct spu_type_subtype *matched_spu_type;
>> + void __iomem *spu_reg_vbase[MAX_SPUS];
>> + int i;
>> + int err;
>> +
>> + if (!of_device_is_available(dn)) {
>> + dev_crit(dev, "SPU device not available");
>> + return -ENODEV;
>> + }
> How can this happen?
You are correct. This is unnecessary. I will remove.
>
>> + /* Count number of mailbox channels */
>> + spu->num_chan = of_count_phandle_with_args(dn, "mboxes", "#mbox-cells");
>> + dev_dbg(dev, "Device has %d SPU channels", spu->num_chan);
>> +
>> + match = of_match_device(of_match_ptr(bcm_spu_dt_ids), dev);
>> + matched_spu_type = (struct spu_type_subtype *)match->data;
> This cast usn't necessary.
Ok, will remove.
>
>> + spu->spu_type = matched_spu_type->type;
>> + spu->spu_subtype = matched_spu_type->subtype;
>> +
>> + /* Read registers and count number of SPUs */
>> + i = 0;
>> + while ((i < MAX_SPUS) && ((spu_ctrl_regs =
>> + platform_get_resource(pdev, IORESOURCE_MEM, i)) != NULL)) {
>> + dev_dbg(dev,
>> + "SPU %d control register region res.start = %#x, res.end = %#x",
>> + i,
>> + (unsigned int)spu_ctrl_regs->start,
>> + (unsigned int)spu_ctrl_regs->end);
>> +
>> + spu_reg_vbase[i] = devm_ioremap_resource(dev, spu_ctrl_regs);
>> + if (IS_ERR(spu_reg_vbase[i])) {
>> + err = PTR_ERR(spu_reg_vbase[i]);
>> + dev_err(&pdev->dev, "Failed to map registers: %d\n",
>> + err);
>> + spu_reg_vbase[i] = NULL;
>> + return err;
>> + }
>> + i++;
>> + }
> These *really* sound like independent devices. There are no shared
> registers, and each has its own mbox.
>
> Why do we group them like this?
As I said in the previous email, I want one instance of the driver to
register crypto algos once with the crypto API and to distribute crypto
requests among all available SPU hw blocks.
>
> Thanks,
> Mark.
^ permalink raw reply
* [Question] New mmap64 syscall?
From: Yury Norov @ 2016-12-07 15:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f544bcc3-8aae-1bd0-b744-9964fc038a51@redhat.com>
On Wed, Dec 07, 2016 at 02:23:55PM +0100, Florian Weimer wrote:
> On 12/06/2016 07:54 PM, Yury Norov wrote:
> >3. Introduce new mmap64() syscall like this:
> >sys_mmap64(void *addr, size_t len, int prot, int flags, int fd, struct off_pair *off);
> >(The pointer here because otherwise we have 7 args, if simply pass off_hi and
> >off_lo in registers.)
>
> I would prefer a batched mmap/munmap/mremap/mprotect/madvise interface, so
> that VM changes can be coalesced and the output reduced. This interface
> could then be used to implement mmap on 32-bit architectures as well because
> the offset restrictions would not apply there.
Hi Florian,
I frankly don't understand what you mean, All syscalls you mentioned
doesn't take off_t or other 64-bit arguments. 'VM changes' - virtual
memory? If so, I don't see any changes in VM with this approach, just
correct handling of big offsets.
> This interface
> could then be used to implement mmap on 32-bit architectures as well
This is for 32-bit architectures only. 64 bit arches use
sysdeps/unix/sysv/linux/wordsize-64/mmap.c for both mmap and mmap64,
and they don't need that tricks with off_t. Or you meaning to switch
64-bit mmap to this interface?
Please explain what you mean in details.
Yury
^ permalink raw reply
* [PATCH v4 1/4] [media] davinci: vpif_capture: don't lock over s_stream
From: Laurent Pinchart @ 2016-12-07 15:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <m237i1gfz1.fsf@baylibre.com>
Hi Kevin,
On Tuesday 06 Dec 2016 08:49:38 Kevin Hilman wrote:
> Laurent Pinchart writes:
> > On Tuesday 29 Nov 2016 15:57:09 Kevin Hilman wrote:
> >> Video capture subdevs may be over I2C and may sleep during xfer, so we
> >> cannot do IRQ-disabled locking when calling the subdev.
> >>
> >> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> >> ---
> >> drivers/media/platform/davinci/vpif_capture.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/media/platform/davinci/vpif_capture.c
> >> b/drivers/media/platform/davinci/vpif_capture.c index
> >> 5104cc0ee40e..9f8f41c0f251 100644
> >> --- a/drivers/media/platform/davinci/vpif_capture.c
> >> +++ b/drivers/media/platform/davinci/vpif_capture.c
> >> @@ -193,7 +193,10 @@ static int vpif_start_streaming(struct vb2_queue
> >> *vq, unsigned int count)
> >> }
> >> }
> >>
> >> + spin_unlock_irqrestore(&common->irqlock, flags);
> >> ret = v4l2_subdev_call(ch->sd, video, s_stream, 1);
> >> + spin_lock_irqsave(&common->irqlock, flags);
> >
> > I always get anxious when I see a spinlock being released randomly with an
> > operation in the middle of a protected section. Looking at the code it
> > looks like the spinlock is abused here. irqlock should only protect the
> > dma_queue and should thus only be taken around the following code:
> >
> > spin_lock_irqsave(&common->irqlock, flags);
> > /* Get the next frame from the buffer queue */
> > common->cur_frm = common->next_frm = list_entry(common->dma_queue.next,
> > struct vpif_cap_buffer, list);
> >
> > /* Remove buffer from the buffer queue */
> > list_del(&common->cur_frm->list);
> > spin_unlock_irqrestore(&common->irqlock, flags);
>
> Yes, that looks correct. Will update.
>
> > The code that is currently protected by the lock in the start and stop
> > streaming functions should be protected by a mutex instead.
>
> I tried taking the mutex here, but lockdep pointed out a deadlock. I
> may not be fully understanding the V4L2 internals here, but it seems
> that the ioctl is already taking a mutex, so taking it again in
> start/stop streaming is a deadlock. Unless you think the locking should
> be nested here, it seems to me that the mutex isn't needed.
The V4L2 core can lock all ioctls using struct video_device::lock. For buffer-
related ioctls, it can optionally use a separate lock from struct
vb2_queue::lock. See v4l2_ioctl_get_lock() in drivers/media/v4l2-core/v4l2-
ioctl.c.
The vpif-capture driver sets both the video_device and vb2_queue locks to the
same lock (which would have the same effect as leaving the vb2_queue lock
NULL). All ioctls are thus serialized. You would only need to handle locking
in start_streaming and stop_streaming manually if you didn't rely on the core
serializing the ioctls.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH 1/3] crypto: brcm: DT documentation for Broadcom SPU driver
From: Rob (William) Rice @ 2016-12-07 15:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206140607.GB24177@leverpostej>
Mark,
Thanks for your comments. Replies below.
Rob
On 12/6/2016 9:06 AM, Mark Rutland wrote:
> On Wed, Nov 30, 2016 at 03:07:31PM -0500, Rob Rice wrote:
>> Device tree documentation for Broadcom Secure Processing Unit
>> (SPU) crypto driver.
>>
>> Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
>> Signed-off-by: Rob Rice <rob.rice@broadcom.com>
>> ---
>> .../devicetree/bindings/crypto/brcm,spu-crypto.txt | 25 ++++++++++++++++++++++
>> 1 file changed, 25 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
>>
>> diff --git a/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt b/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
>> new file mode 100644
>> index 0000000..e5fe942
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
>> @@ -0,0 +1,25 @@
>> +The Broadcom Secure Processing Unit (SPU) driver supports symmetric
>> +cryptographic offload for Broadcom SoCs with SPU hardware. A SoC may have
>> +multiple SPU hardware blocks.
> Bindings shound describe *hardware*, not *drivers*. Please drop mention
> of the driver, and just decribe the hardware.
Makes sense. I'll change it.
>
>> +Required properties:
>> +- compatible : Should be "brcm,spum-crypto" for devices with SPU-M hardware
>> + (e.g., Northstar2) or "brcm,spum-nsp-crypto" for the Northstar Plus variant
>> + of the SPU-M hardware.
>> +
>> +- reg: Should contain SPU registers location and length.
>> +- mboxes: A list of mailbox channels to be used by the kernel driver. Mailbox
>> +channels correspond to DMA rings on the device.
>> +
>> +Example:
>> + spu-crypto at 612d0000 {
>> + compatible = "brcm,spum-crypto";
>> + reg = <0 0x612d0000 0 0x900>, /* SPU 0 control regs */
>> + <0 0x612f0000 0 0x900>, /* SPU 1 control regs */
>> + <0 0x61310000 0 0x900>, /* SPU 2 control regs */
>> + <0 0x61330000 0 0x900>; /* SPU 3 control regs */
> The above didn't mention there were several register sets, and the
> comment beside each makes them sound like they're separate SPU
> instances, so I don't think it makes sense to group them as one node.
>
> What's going on here?
That's right. For the SoC I used as the example, there are four SPU
hardware blocks. The driver round robins crypto requests to the hardware
blocks to handle requests in parallel and increase throughput. I want
one instance of the SPU driver that registers algos once with the crypto
API and manages all the mailbox channels. Maybe I can achieve that with
separate device tree entries for each SPU block, I'm not sure. I'll look
into it.
>
>> + mboxes = <&pdc0 0>,
>> + <&pdc1 0>,
>> + <&pdc2 0>,
>> + <&pdc3 0>;
> Does each mbox correspond to one of the SPUs above? Or is there a shared
> pool?
Yes, each of these mailbox channels corresponds to a different SPU. PDC
is a DMA ring manager for DMAs to the SPUs.
>
> Thanks,
> Mark.
^ permalink raw reply
* [PATCH] ARM: dts: armada-{370, xp}: drop "marvell, orion-spi" from SPI controllers
From: Uwe Kleine-König @ 2016-12-07 15:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87eg1jhi4l.fsf@free-electrons.com>
Hello Gregory,
On Wed, Dec 07, 2016 at 04:30:02PM +0100, Gregory CLEMENT wrote:
> On mer., d?c. 07 2016, Uwe Kleine-K?nig <uwe@kleine-koenig.org> wrote:
>
> > From: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> >
> > The SPI controllers on Armada 370 and XP differ from the original Orion
> > SPI controllers (at least) in the configuration of the baud rate. So
> > it's wrong to claim compatibility which results in bogus baud rates.
>
> Until two years ago with the commits
> df59fa7f4bca9658b75f0f5fee225b3a057475c5 and
> 4dacccfac69494ba70248b134352f299171c41b7
> we used "marvell,orion-spi" compatible on Armada XP and Armada 370
> without any problem.
>
> The new compatible "marvell,armada-xp-spi" and "marvell,armada-xp-spi"
> allows to have more choice for the baudrate for a given clock but it is
> not true that Armada 370 and Armada XP are not compatible with
> "marvell,orion-spi".
The issue I was faced with that made me create this patch is that in
barebox no special case for 370/XP was active. The result was that a
device that could be operated at 60 MHz only got a clock of 11 MHz and
the driver assumed it configured 41.666 MHz. I didn't check if the same
can happen in the opposite direction (or if there are other important
incompatibilities) but still I'd say this is a bug with my patch being
the obvious fix.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH] drm: hdlcd: Work properly in big-endian mode
From: Robin Murphy @ 2016-12-07 15:31 UTC (permalink / raw)
To: linux-arm-kernel
Under a big-endian kernel, colours on the framebuffer all come out a
delightful shade of wrong, since we fail to take the reversed byte
order into account. Fortunately, the HDLCD has a control bit to make it
automatically byteswap big-endian data; let's use it as appropriate.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/gpu/drm/arm/hdlcd_crtc.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 28341b32067f..eceb7bed5dd0 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -63,6 +63,7 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
uint32_t pixel_format;
struct simplefb_format *format = NULL;
int i;
+ u32 reg;
pixel_format = crtc->primary->state->fb->pixel_format;
@@ -76,7 +77,11 @@ static int hdlcd_set_pxl_fmt(struct drm_crtc *crtc)
/* HDLCD uses 'bytes per pixel', zero means 1 byte */
btpp = (format->bits_per_pixel + 7) / 8;
- hdlcd_write(hdlcd, HDLCD_REG_PIXEL_FORMAT, (btpp - 1) << 3);
+ reg = (btpp - 1) << 3;
+#ifdef __BIG_ENDIAN
+ reg |= HDLCD_PIXEL_FMT_BIG_ENDIAN;
+#endif
+ hdlcd_write(hdlcd, HDLCD_REG_PIXEL_FORMAT, reg);
/*
* The format of the HDLCD_REG_<color>_SELECT register is:
--
2.10.2.dirty
^ permalink raw reply related
* [PATCH] ARM: dts: armada-{370, xp}: drop "marvell, orion-spi" from SPI controllers
From: Gregory CLEMENT @ 2016-12-07 15:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161207152109.17545-1-uwe@kleine-koenig.org>
Hi Uwe,
On mer., d?c. 07 2016, Uwe Kleine-K?nig <uwe@kleine-koenig.org> wrote:
> From: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
>
> The SPI controllers on Armada 370 and XP differ from the original Orion
> SPI controllers (at least) in the configuration of the baud rate. So
> it's wrong to claim compatibility which results in bogus baud rates.
Until two years ago with the commits
df59fa7f4bca9658b75f0f5fee225b3a057475c5 and
4dacccfac69494ba70248b134352f299171c41b7
we used "marvell,orion-spi" compatible on Armada XP and Armada 370
without any problem.
The new compatible "marvell,armada-xp-spi" and "marvell,armada-xp-spi"
allows to have more choice for the baudrate for a given clock but it is
not true that Armada 370 and Armada XP are not compatible with
"marvell,orion-spi".
Gregory
>
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> ---
> arch/arm/boot/dts/armada-370.dtsi | 4 ++--
> arch/arm/boot/dts/armada-xp.dtsi | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
> index b4258105e91f..b9377c11b379 100644
> --- a/arch/arm/boot/dts/armada-370.dtsi
> +++ b/arch/arm/boot/dts/armada-370.dtsi
> @@ -435,13 +435,13 @@
> * board level if a different configuration is used.
> */
> &spi0 {
> - compatible = "marvell,armada-370-spi", "marvell,orion-spi";
> + compatible = "marvell,armada-370-spi";
> pinctrl-0 = <&spi0_pins1>;
> pinctrl-names = "default";
> };
>
> &spi1 {
> - compatible = "marvell,armada-370-spi", "marvell,orion-spi";
> + compatible = "marvell,armada-370-spi";
> pinctrl-0 = <&spi1_pins>;
> pinctrl-names = "default";
> };
> diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
> index 4a5f99e65b51..3a416834d80b 100644
> --- a/arch/arm/boot/dts/armada-xp.dtsi
> +++ b/arch/arm/boot/dts/armada-xp.dtsi
> @@ -367,13 +367,13 @@
> };
>
> &spi0 {
> - compatible = "marvell,armada-xp-spi", "marvell,orion-spi";
> + compatible = "marvell,armada-xp-spi";
> pinctrl-0 = <&spi0_pins>;
> pinctrl-names = "default";
> };
>
> &spi1 {
> - compatible = "marvell,armada-xp-spi", "marvell,orion-spi";
> + compatible = "marvell,armada-xp-spi";
> pinctrl-0 = <&spi1_pins>;
> pinctrl-names = "default";
> };
> --
> 2.10.2
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH v4 4/4] ARM: da850: fix da850_set_pll0rate()
From: Bartosz Golaszewski @ 2016-12-07 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481124138-27337-1-git-send-email-bgolaszewski@baylibre.com>
This function is confusing - its second argument is an index to the
freq table, not the requested clock rate in Hz, but it's used as the
set_rate callback for the pll0 clock. It leads to an oops when the
caller doesn't know the internals and passes the rate in Hz as
argument instead of the cpufreq index since this argument isn't bounds
checked either.
Fix it by iterating over the array of supported frequencies and
selecting a one that matches or returning -EINVAL for unsupported
rates.
Also: update the davinci cpufreq driver. It's the only user of this
clock and currently it passes the cpufreq table index to
clk_set_rate(), which is confusing. Make it pass the requested clock
rate in Hz.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
arch/arm/mach-davinci/da850.c | 22 ++++++++++++++++++----
drivers/cpufreq/davinci-cpufreq.c | 2 +-
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 5f8ffaa..b410598 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -1197,14 +1197,28 @@ static int da850_set_armrate(struct clk *clk, unsigned long index)
return clk_set_rate(pllclk, index);
}
-static int da850_set_pll0rate(struct clk *clk, unsigned long index)
+static int da850_set_pll0rate(struct clk *clk, unsigned long rate)
{
- unsigned int prediv, mult, postdiv;
- struct da850_opp *opp;
struct pll_data *pll = clk->pll_data;
+ struct cpufreq_frequency_table *freq;
+ unsigned int prediv, mult, postdiv;
+ struct da850_opp *opp = NULL;
int ret;
- opp = (struct da850_opp *) cpufreq_info.freq_table[index].driver_data;
+ rate /= 1000;
+
+ for (freq = da850_freq_table;
+ freq->frequency != CPUFREQ_TABLE_END; freq++) {
+ /* rate is in Hz, freq->frequency is in KHz */
+ if (freq->frequency == rate) {
+ opp = (struct da850_opp *)freq->driver_data;
+ break;
+ }
+ }
+
+ if (!opp)
+ return -EINVAL;
+
prediv = opp->prediv;
mult = opp->mult;
postdiv = opp->postdiv;
diff --git a/drivers/cpufreq/davinci-cpufreq.c b/drivers/cpufreq/davinci-cpufreq.c
index b95a872..d54a27c 100644
--- a/drivers/cpufreq/davinci-cpufreq.c
+++ b/drivers/cpufreq/davinci-cpufreq.c
@@ -55,7 +55,7 @@ static int davinci_target(struct cpufreq_policy *policy, unsigned int idx)
return ret;
}
- ret = clk_set_rate(armclk, idx);
+ ret = clk_set_rate(armclk, new_freq * 1000);
if (ret)
return ret;
--
2.9.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox