Re: [PATCH/RFC v2 1/4] v4l: Add metadata buffer type and format

[Sakari Ailus]

Hi Laurent,

On Wed, Jun 22, 2016 at 07:51:06PM +0300, Laurent Pinchart wrote:
> Hello,
> 
> On Tuesday 24 May 2016 19:26:32 Sakari Ailus wrote:
> > On Tue, May 24, 2016 at 05:36:42PM +0200, Hans Verkuil wrote:
> > > On 05/24/2016 05:28 PM, Sakari Ailus wrote:
> > > > Hi Hans,
> > > > 
> > > >> Should it be mentioned here that changing the video format might change
> > > >> the buffersize? In case the buffersize is always a multiple of the
> > > >> width?
> > > > 
> > > > Isn't that the case in general, as with pixel formats? buffersize could
> > > > also be something else than a multiple of width (there's no width for
> > > > metadata formats) due to e.g. padding required by hardware.
> > > 
> > > Well, I don't think it is obvious that the metadata buffersize depends on
> > > the video width. Perhaps developers who are experienced with CSI know
> > > this, but if you know little or nothing about CSI, then it can be
> > > unexpected (hey, that was the case for me!).
> > > 
> > > I think it doesn't hurt to mention this relation.
> > 
> > Ah, I think I misunderstood you first.
> > 
> > Typically the metadata width is the same as the image data width, that's
> > true. And it's how the hardware works. This is still visible in the media
> > bus format and the solution belongs rather to how multiple streams over a
> > single link are supported.
> 
> Let me clarify on this.
> 
> In the general case there's no concept of metadata width when stored in 
> memory. The two most common use cases for metadata store register values (or 
> similar) information, or statistics. The former is just a byte stream in some 

In general case perhaps not.

But in specific cases such as sensor produced embedded data is indeed
line-based, with each line having a preable and a predetermined amount of
data after the preamble. In this respect it's very much like image data.

> kind of TLV (Type Length Value) format. The latter a set of values or arrays 
> computed either on the full image or on subwindows, possibly laid out as a 
> grid.
> 
> When transported over a bus, however, metadata can sometimes have a width and 
> height. That's the case for CSI-2, which is a line-oriented format. Metadata 
> then need to be broken into chunks transmitted in a CSI-2 line packet, even 
> if 
> they don't correspond to a line of an image. The line width on the bus is 
> just 
> the number of bytes transmitted in a single packet, which could be chosen 
> freely (within the range allowed by CSI-2). In practice, to simplify the 
> implementation, the line width is chosen to be identical to the line width of 
> the image frames that the metadata correspond to, but that's not a 
> requirement 
> of either CSI-2 or the metadata format itself.
> 
> We thus need to expose metadata width and height on subdevs to ensure proper 
> configuration of the transmitter and receiver, but that's not strictly 
> mandatory on video nodes.

To support sensor embedded data, I think we do need bytesperline, width and
height. They might not be applicable to e.g. some types of statistics.

The rest looks good to me.

> 
> The metadata buffer size itself doesn't depend on the width and height of the 
> corresponding image frames. A histogram using 64 bins on 3 components will be 
> stored exactly the same way regardless of whether it's computed on a VGA or 
> 1080p frame. The buffer size depends on the configuration of the metadata 
> source only, which in the case of the histogram generator in the VSP would 
> include a control that decides whether to compute the histogram with 64 bins 
> or 256 bins (the latter needs a 4 times larger buffer).
> 
> For metadata computed on a variable number of subwindows the buffer size will 
> depend on the number of subwindows, which will in turn be possibly influenced 
> by the size of the image. It could make sense to use fewer subwindows to 
> compute AF data on a VGA image than on a 4k image. That is not however a 
> requirement, and there's no direct mapping between image size and metadata 
> size, the number of subwindows being usually configured by userspace.
> 
> I hope this clarifies the problem. Please let me know if you have additional 
> questions or thoughts, and if you see anything that should be worded 
> differently in the documentation (or even structures that should get new 
> fields).
> 
> > It's just that setting the image media bus format affects the metadata media
> > bus format. I guess that could be mentioned albeit it's hardware specific,
> > on some sensors metadata width is independent of the image width. Even then
> > this is not where I'd put it. I'd get back to the topic when documenting
> > how the API for multiple streams over a single link works.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH 01/24] v4l: Add metadata buffer type and format

[Guennadi Liakhovetski]

Hi Hans,

On Fri, 24 Jun 2016, Hans Verkuil wrote:

> On 06/24/2016 05:57 PM, Hans Verkuil wrote:
> > On 06/20/2016 09:10 PM, Laurent Pinchart wrote:
> >> The metadata buffer type is used to transfer metadata between userspace
> >> and kernelspace through a V4L2 buffers queue. It comes with a new
> >> metadata capture capability and format description.
> >>
> >> Signed-off-by: Laurent Pinchart 
> > 
> > I am willing to Ack this, provided Sakari and Guennadi Ack this as well. 
> > They know
> > more about metadata handling in various types of hardware than I do, so I 
> > feel
> > their Acks are important here.
> 
> Actually, I would like to see more about how applications can associate 
> frames with
> metadata (if such a correspondence exists).

I think Laurent mentioned this in one of his patches, that the sequence 
number should be used to establish a correspondence, and this is also what 
my UVC patch does.

Thanks
Guennadi

> There was an irc discussion about that here:
> 
> https://linuxtv.org/irc/irclogger_log/v4l?date=2016-06-24,Fri
> 
> Guennadi's uvc patches may be useful as a testbed for figuring this out.
> 
> Regards,
> 
>   Hans
> 

Re: [PATCH 01/24] v4l: Add metadata buffer type and format

[Guennadi Liakhovetski]

On Fri, 24 Jun 2016, Hans Verkuil wrote:

> On 06/20/2016 09:10 PM, Laurent Pinchart wrote:
> > The metadata buffer type is used to transfer metadata between userspace
> > and kernelspace through a V4L2 buffers queue. It comes with a new
> > metadata capture capability and format description.
> > 
> > Signed-off-by: Laurent Pinchart 

Tested-by: Guennadi Liakhovetski 

If an acked-by is desired too, I'll give the patch a second look next 
Monday just to see if I can find a missing empty line to nit-pick on ;-)

Thanks
Guennadi

> I am willing to Ack this, provided Sakari and Guennadi Ack this as well. They 
> know
> more about metadata handling in various types of hardware than I do, so I feel
> their Acks are important here.
> 
> Regards,
> 
>   Hans
> 
> > ---
> >  Documentation/DocBook/media/v4l/dev-meta.xml  | 93 
> > +++
> >  Documentation/DocBook/media/v4l/v4l2.xml  |  1 +
> >  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 19 ++
> >  drivers/media/v4l2-core/v4l2-dev.c| 16 +++--
> >  drivers/media/v4l2-core/v4l2-ioctl.c  | 34 ++
> >  drivers/media/v4l2-core/videobuf2-v4l2.c  |  3 +
> >  include/media/v4l2-ioctl.h|  8 +++
> >  include/uapi/linux/videodev2.h| 14 
> >  8 files changed, 182 insertions(+), 6 deletions(-)
> >  create mode 100644 Documentation/DocBook/media/v4l/dev-meta.xml
> > 
> > diff --git a/Documentation/DocBook/media/v4l/dev-meta.xml 
> > b/Documentation/DocBook/media/v4l/dev-meta.xml
> > new file mode 100644
> > index ..9b5b1fba2007
> > --- /dev/null
> > +++ b/Documentation/DocBook/media/v4l/dev-meta.xml
> > @@ -0,0 +1,93 @@
> > +  Metadata Interface
> > +
> > +  
> > +Experimental
> > +This is an  experimental 
> > +interface and may change in the future.
> > +  
> > +
> > +  
> > +Metadata refers to any non-image data that supplements video frames with
> > +additional information. This may include statistics computed over the image
> > +or frame capture parameters supplied by the image source. This interface is
> > +intended for transfer of metadata to userspace and control of that 
> > operation.
> > +  
> > +
> > +  
> > +The metadata interface is implemented on video capture devices. The device 
> > can
> > +be dedicated to metadata or can implement both video and metadata capture 
> > as
> > +specified in its reported capabilities.
> > +  
> > +
> > +  
> > +Querying Capabilities
> > +
> > +
> > +Devices supporting the metadata interface set the
> > +V4L2_CAP_META_CAPTURE flag in the
> > +capabilities field of 
> > +returned by the  ioctl. That flag means the device can 
> > capture
> > +metadata to memory.
> > +
> > +
> > +At least one of the read/write or streaming I/O methods must be supported.
> > +
> > +  
> > +
> > +  
> > +Data Format Negotiation
> > +
> > +
> > +The metadata device uses the format ioctls to
> > +select the capture format. The metadata buffer content format is bound to 
> > that
> > +selectable format. In addition to the basic
> > +format ioctls, the  ioctl
> > +must be supported as well.
> > +
> > +
> > +
> > +To use the format ioctls applications set the
> > +type field of a  to
> > +V4L2_BUF_TYPE_META_CAPTURE and use the 
> > 
> > +meta member of the 
> > fmt
> > +union as needed per the desired operation.
> > +Currently there are two fields, dataformat and
> > +buffersize, of struct  that 
> > are
> > +used. Content of the dataformat is the V4L2 
> > FourCC
> > +code of the data format. The buffersize field 
> > is the
> > +maximum buffer size in bytes required for data transfer, set by the driver 
> > in
> > +order to inform applications.
> > +
> > +
> > +
> > +  struct v4l2_meta_format
> > +  
> > +
> > +
> > +  
> > +__u32
> > +dataformat
> > +
> > +The data format, set by the application. This is a little endian
> > +four character code.
> > +V4L2 defines metadata formats in .
> > +   
> > +  
> > +  
> > +__u32
> > +buffersize
> > +
> > +Maximum size in bytes required for data. Value is set by the driver.
> > +   
> > +  
> > +  
> > +__u8
> > +reserved[24]
> > +This array is reserved for future extensions.
> > +Drivers and applications must set it to zero.
> > +  
> > +
> > +  
> > +
> > +
> > +  
> > diff --git a/Documentation/DocBook/media/v4l/v4l2.xml 
> > b/Documentation/DocBook/media/v4l/v4l2.xml
> > index 42e626d6c936..5c83b5d342dd 100644
> > --- a/Documentation/DocBook/media/v4l/v4l2.xml
> > +++ b/Documentation/DocBook/media/v4l/v4l2.xml
> > @@ -605,6 +605,7 @@ and discussions on the V4L mailing list.
> >
> >
> >
> > +  
> >
> >
> >

Re: [PATCH/RFC 1/6] spi: Document DT bindings for SPI controllers in slave mode

[Rob Herring]

On Wed, Jun 22, 2016 at 03:42:04PM +0200, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven 
> ---
>  Documentation/devicetree/bindings/spi/spi-bus.txt | 31 
> ++-
>  1 file changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt 
> b/Documentation/devicetree/bindings/spi/spi-bus.txt
> index 17822860cb98c34d..bcd024e0bb9e0009 100644
> --- a/Documentation/devicetree/bindings/spi/spi-bus.txt
> +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
> @@ -1,17 +1,23 @@
>  SPI (Serial Peripheral Interface) busses
>  
> -SPI busses can be described with a node for the SPI master device
> -and a set of child nodes for each SPI slave on the bus.  For this
> -discussion, it is assumed that the system's SPI controller is in
> -SPI master mode.  This binding does not describe SPI controllers
> -in slave mode.
> +SPI busses can be described with a node for the SPI controller device
> +and a set of child nodes for each SPI slave on the bus.  The system's SPI
> +controller may be described for use in SPI master mode or in SPI slave mode,
> +but not for both at the same time.
>  
> -The SPI master node requires the following properties:
> +The SPI controller node requires the following properties:
> +- compatible  - name of SPI bus controller following generic names
> + recommended practice.
> +
> +In master mode, the SPI controller node requires the following additional
> +properties:
>  - #address-cells  - number of cells required to define a chip select
>   address on the SPI bus.
>  - #size-cells - should be zero.
> -- compatible  - name of SPI bus controller following generic names
> - recommended practice.
> +
> +In slave node, the SPI controller node requires the presence of a child node
> +named "slave", further following the practices for SPI slave nodes below.
> +

I wouldn't create a child node. Just add a property for slave mode and 
put the timing mode properties in controller node.

Rob

Re: [PATCH 01/24] v4l: Add metadata buffer type and format

[Hans Verkuil]

On 06/24/2016 05:57 PM, Hans Verkuil wrote:
> On 06/20/2016 09:10 PM, Laurent Pinchart wrote:
>> The metadata buffer type is used to transfer metadata between userspace
>> and kernelspace through a V4L2 buffers queue. It comes with a new
>> metadata capture capability and format description.
>>
>> Signed-off-by: Laurent Pinchart 
> 
> I am willing to Ack this, provided Sakari and Guennadi Ack this as well. They 
> know
> more about metadata handling in various types of hardware than I do, so I feel
> their Acks are important here.

Actually, I would like to see more about how applications can associate frames 
with
metadata (if such a correspondence exists).

There was an irc discussion about that here:

https://linuxtv.org/irc/irclogger_log/v4l?date=2016-06-24,Fri

Guennadi's uvc patches may be useful as a testbed for figuring this out.

Regards,

Hans

Re: [PATCH v2 1/2] can: rcar_canfd: Add Classical CAN only mode support

[Rob Herring]

On Wed, Jun 22, 2016 at 01:31:46PM +0100, Ramesh Shanmugasundaram wrote:
> The controller can operate in one of the two global modes
>  - CAN FD only mode (default)
>  - Classical CAN (CAN2.0) only mode
> 
> This patch adds support for Classical CAN only mode. It can be enabled
> by defining the optional device tree property "renesas,no-can-fd" of this
> node.
> 
> Note: R-Car Gen3 h/w manual v0.51E shows bit6 of RSCFDnCFDGCFG as
> reserved, which is incorrect. This bit is same as RSCFDnGCFG.
> 
> Signed-off-by: Ramesh Shanmugasundaram 
> 
> ---
>  .../devicetree/bindings/net/can/rcar_canfd.txt |  21 +-

Acked-by: Rob Herring 

>  drivers/net/can/rcar/rcar_canfd.c  | 355 
> ++---
>  2 files changed, 255 insertions(+), 121 deletions(-)

[PATCH v2 2/4] serial: sh-sci: Stop transfers in sci_shutdown()

[Geert Uytterhoeven]

Make sure the transmitter and receiver are stopped when shutting down
the port, and related interrupts are disabled.

Without this:
  - New input data may be received into the RX FIFO, possibly
triggering a new RX DMA completion,
  - Transfers will still be enabled on a subsequent startup of the UART,
before the UART's FIFOs have been reset, causing reading of stale
data.

Inspired by a patch in the BSP by Koji Matsuoka
.

Signed-off-by: Geert Uytterhoeven 
---
v2:
  - Do not clear clock source bits, since the serial console relies on
them, as reported by Shimoda-san.

v1 was extracted from "[PATCH/RFC v3 0/4] serial: sh-sci: Add DT DMA
support".  The issues with the serial console seen before on
r8a7740/armadillo and sh73a0/kzm9g seem to be gone.  Changes after
resurrection:
  - Write zero to also disable related interrupts, as suggested by
Laurent Pinchart,
  - Enhanced patch description.
---
 drivers/tty/serial/sh-sci.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index d6ba90c572f7475c..eecace576c3b758f 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1890,12 +1890,17 @@ static void sci_shutdown(struct uart_port *port)
 {
struct sci_port *s = to_sci_port(port);
unsigned long flags;
+   u16 scr;
 
dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);
 
spin_lock_irqsave(>lock, flags);
sci_stop_rx(port);
sci_stop_tx(port);
+   /* Stop RX and TX, disable related interrupts, keep clock source */
+   scr = serial_port_in(port, SCSCR);
+   serial_port_out(port, SCSCR, scr & (SCSCR_CKE1 | SCSCR_CKE0));
+
spin_unlock_irqrestore(>lock, flags);
 
 #ifdef CONFIG_SERIAL_SH_SCI_DMA
-- 
1.9.1

[PATCH v2 0/4] serial: sh-sci: FIFO initialization fixes

[Geert Uytterhoeven]

Hi Greg, Jiri,

When opening a Renesas SCIF serial port after it has been used before,
stale data may be read.  This has been observed on R-Car Gen2 and R-Car
Gen3, with all SCIF variants present (SCIF, SCIFA, SCIFB, and HSCIF). It
is much more likely to happen when DMA is enabled, although it has been
seen with PIO, too.

There are actually two reasons why stale data is received:
  1. Transfers are started, or are still activated, before the FIFO is
 reset, causing one or more (up to the RX FIFO size) stale bytes
 (fixed by patches 1 and 2),
  2. FIFO reset lacked clearing the RDF flag, causing one byte of stale
 data (fixed by patch 3).
While at it, patch 4 adds the missing clearing of two other flags in the
initialization sequence on (H)SCIF.

While the issue can be reproduced using subsequent runs of sertest[1],
I wrote a new test program, fifotest[2], to trigger it more easily.
More detailed test information can be found on the eLinux wiki[3].

I have verified that this series fixes the issue on SCIF, SCIFA, SCIFB,
and HSCIF, on R-Car Gen2 (r8a7791/koelsch) and R-Car Gen3
(r8a7795/salvator-x).
Basic regression testing has been done with SCIFA on sh73a0/kzm9g.
Regression testing on other variants (notably SCI) would be appreciated.

Changes compared to v1:
  - Do not clear clock source bits, since the serial console relies on
them, as reported by Shimoda-san.

This series applies against both v4.6 and next-20160526.
For testing, it's also available in a topic branch at
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/log/?h=topic/scif-fifo-v2.

Please apply, thanks!

[1] https://github.com/geertu/sertest
[2] https://github.com/geertu/fifotest
[3] http://elinux.org/Tests:SCIF-FIFO

Geert Uytterhoeven (4):
  serial: sh-sci: Do not start transfers from sci_startup()
  serial: sh-sci: Stop transfers in sci_shutdown()
  serial: sh-sci: Clear RX, error, and break flags during reset
  serial: sh-sci: Clear (H)SCIF timeout and overrun during reset

 drivers/tty/serial/sh-sci.c | 20 ++--
 drivers/tty/serial/sh-sci.h |  1 +
 2 files changed, 15 insertions(+), 6 deletions(-)

-- 
1.9.1

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

[PATCH v2 1/4] serial: sh-sci: Do not start transfers from sci_startup()

[Geert Uytterhoeven]

FIFO reset is done in sci_reset(), called from sci_set_termios(), while
sci_start_tx() and sci_start_rx() are called before, from sci_startup().
However, starting transfers before the UART's FIFOs have been reset may
cause reading of stale data.

Remove the calls to sci_start_tx() and sci_start_rx() from sci_startup()
to fix this.

Transfers are still started when needed:
  - sci_start_rx() is called from sci_set_termios() after FIFO reset, if
the CREAD flag is set,
  - sci_start_tx() is called from uart_change_speed() immediately
thereafter, if transmission is enabled.

Signed-off-by: Geert Uytterhoeven 
---
v2:
  - No changes.
---
 drivers/tty/serial/sh-sci.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 0130feb069aee02f..d6ba90c572f7475c 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1873,7 +1873,6 @@ static void sci_break_ctl(struct uart_port *port, int 
break_state)
 static int sci_startup(struct uart_port *port)
 {
struct sci_port *s = to_sci_port(port);
-   unsigned long flags;
int ret;
 
dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);
@@ -1884,11 +1883,6 @@ static int sci_startup(struct uart_port *port)
 
sci_request_dma(port);
 
-   spin_lock_irqsave(>lock, flags);
-   sci_start_tx(port);
-   sci_start_rx(port);
-   spin_unlock_irqrestore(>lock, flags);
-
return 0;
 }
 
-- 
1.9.1

[PATCH v2 3/4] serial: sh-sci: Clear RX, error, and break flags during reset

[Geert Uytterhoeven]

Setting the FIFO reset bits is not sufficient to reset the RX FIFO.
After this the status register's RDF flag bit may still be set, causing
the reception of one stale byte of data.

To fix this, clear all status flag bits related to reception, error, and
break handling, cfr. the initialization flowchart in the datasheet.

Signed-off-by: Geert Uytterhoeven 
---
v2:
  - No changes.
---
 drivers/tty/serial/sh-sci.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index eecace576c3b758f..0a2be5d293220e08 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2055,6 +2055,10 @@ static void sci_reset(struct uart_port *port)
reg = sci_getreg(port, SCFCR);
if (reg->size)
serial_port_out(port, SCFCR, SCFCR_RFRST | SCFCR_TFRST);
+
+   sci_clear_SCxSR(port,
+   SCxSR_RDxF_CLEAR(port) & SCxSR_ERROR_CLEAR(port) &
+   SCxSR_BREAK_CLEAR(port));
 }
 
 static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
-- 
1.9.1

[PATCH v2 4/4] serial: sh-sci: Clear (H)SCIF timeout and overrun during reset

[Geert Uytterhoeven]

Add the missing timeout bit definition for (H)SCIF.
Clear the timeout and overrun flag bits during UART reset, cfr. the
initialization flowchart in the datasheet.

Signed-off-by: Geert Uytterhoeven 
---
v2:
  - No changes.
---
 drivers/tty/serial/sh-sci.c | 5 +
 drivers/tty/serial/sh-sci.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 0a2be5d293220e08..cf2a7903d2f0270b 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2059,6 +2059,11 @@ static void sci_reset(struct uart_port *port)
sci_clear_SCxSR(port,
SCxSR_RDxF_CLEAR(port) & SCxSR_ERROR_CLEAR(port) &
SCxSR_BREAK_CLEAR(port));
+   if (sci_getreg(port, SCLSR)->size) {
+   status = serial_port_in(port, SCLSR);
+   status &= ~(SCLSR_TO | SCLSR_ORER);
+   serial_port_out(port, SCLSR, status);
+   }
 }
 
 static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
diff --git a/drivers/tty/serial/sh-sci.h b/drivers/tty/serial/sh-sci.h
index 7a4fa185b93ef307..c590418d2a40d78b 100644
--- a/drivers/tty/serial/sh-sci.h
+++ b/drivers/tty/serial/sh-sci.h
@@ -105,6 +105,7 @@ enum {
 #define SCFCR_LOOP BIT(0)  /* Loopback Test */
 
 /* SCLSR (Line Status Register) on (H)SCIF */
+#define SCLSR_TO   BIT(2)  /* Timeout */
 #define SCLSR_ORER BIT(0)  /* Overrun Error */
 
 /* SCSPTR (Serial Port Register), optional */
-- 
1.9.1

Re: [PATCH/RFC v2 4/4] v4l: vsp1: Add HGO support

[Laurent Pinchart]

Hi Guennadi,

On Monday 13 Jun 2016 17:33:01 Guennadi Liakhovetski wrote:
> On Thu, 12 May 2016, Laurent Pinchart wrote:
> > The HGO is a Histogram Generator One-Dimension. It computes per-channel
> > histograms over a configurable region of the image with optional
> > subsampling.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> 
> [snip]
> 
> Do I understand this correctly, that with this if such a metadata device
> is opened, while no video data is streaming, all calls will succeed, but
> no buffers will be dequeued, i.e. the user-space application will hang in
> DQBUF or poll() / select() indefinitely?

That's correct. Same as with a mem-to-mem device, if you open the capture 
node, queue buffers and start streaming, DQBUF will wait until you start 
supplying buffers on the node at the other end of the pipeline.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 19/25] PCI: rcar Gen2: Request host bridge window resources

[Geert Uytterhoeven]

Hi Bjorn,

On Tue, Jun 21, 2016 at 6:49 PM, Bjorn Helgaas  wrote:
> On Tue, Jun 21, 2016 at 06:41:00PM +0300, Valentine Barshak wrote:
>> On Tue, Jun 21, 2016 at 09:26:23AM -0500, Bjorn Helgaas wrote:
>> > On Tue, Jun 21, 2016 at 12:41:31PM +0200, Geert Uytterhoeven wrote:
>> > > On Tue, Jun 7, 2016 at 1:07 AM, Bjorn Helgaas  
>> > > wrote:
>> > > > Request host bridge window resources so they appear in ioport_resource 
>> > > > and
>> > > > iomem_resource and are reflected in /proc/ioports and /proc/iomem.
>> > > >
>> > > > Signed-off-by: Bjorn Helgaas 
>> > > > ---
>> > > >  drivers/pci/host/pci-rcar-gen2.c |4 
>> > > >  1 file changed, 4 insertions(+)
>> > > >
>> > > > diff --git a/drivers/pci/host/pci-rcar-gen2.c 
>> > > > b/drivers/pci/host/pci-rcar-gen2.c
>> > > > index 9980a4b..617a6b2 100644
>> > > > --- a/drivers/pci/host/pci-rcar-gen2.c
>> > > > +++ b/drivers/pci/host/pci-rcar-gen2.c
>> > > > @@ -194,6 +194,7 @@ static int rcar_pci_setup(int nr, struct 
>> > > > pci_sys_data *sys)
>> > > > struct rcar_pci_priv *priv = sys->private_data;
>> > > > void __iomem *reg = priv->reg;
>> > > > u32 val;
>> > > > +   int ret;
>> > > >
>> > > > pm_runtime_enable(priv->dev);
>> > > > pm_runtime_get_sync(priv->dev);
>> > > > @@ -275,6 +276,9 @@ static int rcar_pci_setup(int nr, struct 
>> > > > pci_sys_data *sys)
>> > > > /* Add PCI resources */
>> > > > pci_add_resource(>resources, >io_res);
>> > > > pci_add_resource(>resources, >mem_res);
>> > > > +   ret = devm_request_pci_bus_resources(priv->dev, 
>> > > > >resources);
>> > > > +   if (ret < 0)
>> > > > +   return ret;
>> > > >
>> > > > /* Setup bus number based on platform device id / of bus-range 
>> > > > */
>> > > > sys->busnr = priv->busnr;
>> > >
>> > > This patch (commit 1bd019707b7c9249d34c5d348f1ef75eb4d83e89 in pci/next)
>> > > broke PCI on r8a7791/koelsch. Dmesg differences are:
>> > >
>> > >  pci-rcar-gen2 ee09.pci: PCI: bus0 revision 11
>> > > -pci-rcar-gen2 ee09.pci: PCI host bridge to bus :00
>> > > -pci_bus :00: root bus resource [io  0xee08-0xee0810ff]
>> > > -pci_bus :00: root bus resource [mem 0xee08-0xee0810ff]
>> >
>> > This is probably a result of this code in drivers/pci/host/pci-rcar-gen2.c:
>> >
>> > /*
>> >  * The controller does not support/use port I/O,
>> >  * so setup a dummy port I/O region here.
>> >  */
>> > priv->io_res.start = priv->mem_res.start;
>> > priv->io_res.end = priv->mem_res.end;
>> > priv->io_res.flags = IORESOURCE_IO;
>> >
>> > We try to avoid adding dummy regions like this, but maybe we missed
>> > this one.  I haven't found any email discussion about it yet, so I
>> > don't know what the reason for this one is.  Valentine, do you
>> > remember?
>>
>> I do not, but I think I've found something in my mailbox.
>> Below is the quote from the original conversation with:
>> (Re: [PATCH] pci: Add R-Car Gen2 internal PCI support)
>>
>> [quote starts]
>>  >>+priv->mem_res = *mem_res;
>>  >>+/*
>>  >>+ * The controller does not support/use port I/O,
>>  >>+ * so setup a dummy port I/O region here.
>>  >>+ */
>>  >>+priv->io_res.start = priv->mem_res.start;
>>  >>+priv->io_res.end = priv->mem_res.end;
>>  >>+priv->io_res.flags = IORESOURCE_IO;
>>  >>
>>  > I don't understand this.  There's no requirement (at least as far as the
>>  > PCI core is concerned) to supply an I/O aperture at all, and I think it
>>  > would be better if you didn't.
>>  >
>>  > Oh, I see ... maybe pcibios_init_resources() forces you to have an
>>  > I/O resource to avoid having it give you a default one?  And I
>>  > suppose that since you have several host bridges, these dummy I/O
>>  > regions have to be distinct.  Ugh.  Well, I guess this is something
>>  > you'd have to fix here or in the ARM code, it's up to you what to do.
>>
>>  Exactly. This is to avoid assigning default I/O resources.
>> [quote ends]
>
> Oh, right, now I remember.  Thanks for digging that out.
>
> I propose the patches below to remove the requirement for having an
> I/O space.  Any chance one of you could test them?

Thanks!
PCI seems to be working again on r8a7791/koelsch using today's pci/next,
which includes these patches. Lspci shows the USB controllers again.

/proc/iomem gained a few entries:

 ec70-ec70 : /dma-controller@ec70
 ec72-ec72 : /dma-controller@ec72
 ec74-ec7401ff : audmapp
+ee08-ee0810ff : /pci@ee09
+  ee08-ee080fff : :00:01.0
+  ee081000-ee0810ff : :00:02.0
 ee09-ee090bff : /pci@ee09
+ee0c-ee0c10ff : /pci@ee0d
+  ee0c-ee0c0fff : 0001:01:01.0
+  ee0c1000-ee0c10ff : 0001:01:02.0
 ee0d-ee0d0bff : /pci@ee0d
 ee30-ee301fff : /sata@ee30
 ee70-ee7003ff : /ethernet@ee70

Hence

Re: [PATCH v4 07/13] ARM: shmobile: apmu: Add APMU DT support via Enable method

[Geert Uytterhoeven]

Hi Sergei,

On Tue, Jun 21, 2016 at 7:49 PM, Sergei Shtylyov
 wrote:
> On 06/16/2016 01:27 PM, Geert Uytterhoeven wrote:
>> From: Magnus Damm 
>> Allow DT configuration of the APMU hardware in the case when the APMU is
>> pointed out in the DTB via the enable-method. The ability to configure
>> the APMU via C code is still kept intact to prevent DTB breakage for older
>> SoCs that do not rely on the enable-method for SMP support.
>>
>> Signed-off-by: Magnus Damm 
>> [geert: Fix CONFIG_SMP=n build]
>> Signed-off-by: Geert Uytterhoeven 
>
> [...]
>
>> diff --git a/arch/arm/mach-shmobile/platsmp-apmu.c
>> b/arch/arm/mach-shmobile/platsmp-apmu.c
>> index c1558ef0c590dd3e..0c6bb458b7a45128 100644
>> --- a/arch/arm/mach-shmobile/platsmp-apmu.c
>> +++ b/arch/arm/mach-shmobile/platsmp-apmu.c
>
> [...]
>>
>> +static int shmobile_smp_apmu_boot_secondary_md21(unsigned int cpu,
>> +struct task_struct *idle)
>> +{
>> +   /* Error out when hardware debug mode is enabled */
>> +   if (rcar_gen2_read_mode_pins() & BIT(21)) {
>> +   pr_warn("Unable to boot CPU%u when MD21 is set\n", cpu);
>> +   return -ENOTSUPP;
>> +   }
>> +
>> +   return shmobile_smp_apmu_boot_secondary(cpu, idle);
>> +}
>> +
>> +static struct smp_operations apmu_smp_ops __initdata = {
>> +   .smp_prepare_cpus   = shmobile_smp_apmu_prepare_cpus_dt,
>> +   .smp_boot_secondary = shmobile_smp_apmu_boot_secondary_md21,
>> +#ifdef CONFIG_HOTPLUG_CPU
>> +   .cpu_can_disable= shmobile_smp_cpu_can_disable,
>> +   .cpu_die= shmobile_smp_apmu_cpu_die,
>> +   .cpu_kill   = shmobile_smp_apmu_cpu_kill,
>>  #endif
>
>
>For the record: it turned out that I tested my non-DT SMP on
> R8A7792/Blanche with MD21 bit set. And I've just made sure it still works
> with this implementation (by commenting out the check above.
>Also, I was going to try the workaround for MD21 I saw in the BSP tree --

Which workaround? I only saw the BSP removed the check, but it didn't mention
why (because E2 and V2H don't need it, or...?).

> perhaps it'll help get R8A7791 working w/MD21 set...

Does your Porter work with the other MD21 setting (and the check commented
out, of coutse)?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

RE: [PATCH] usb: renesas_usbhs: make usbhs_write32() static

[Yoshihiro Shimoda]

Hi,

> From: Ben Dooks
> Sent: Wednesday, June 22, 2016 2:53 AM
> 
> The usbhs_write32 function is not used outside of the rcar3.c
> file, so fix the following sparse warning by making it static:
> 
> drivers/usb/renesas_usbhs/rcar3.c:26:6: warning: symbol 'usbhs_write32' was 
> not declared. Should it be static?
> 
> Signed-off-by: Ben Dooks 

Thank you for the patch.

Acked-by: Yoshihiro Shimoda 

Best regards,
Yoshihiro Shimoda

> ---
> Cc: Yoshihiro Shimoda 
> Cc: Greg Kroah-Hartman 
> Cc: linux-...@vger.kernel.org
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> ---
>  drivers/usb/renesas_usbhs/rcar3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/renesas_usbhs/rcar3.c 
> b/drivers/usb/renesas_usbhs/rcar3.c
> index 38b01f2..1d70add 100644
> --- a/drivers/usb/renesas_usbhs/rcar3.c
> +++ b/drivers/usb/renesas_usbhs/rcar3.c
> @@ -23,7 +23,7 @@
>  #define UGCTRL2_RESERVED_3   0x0001  /* bit[3:0] should be B'0001 */
>  #define UGCTRL2_USB0SEL_OTG  0x0030
> 
> -void usbhs_write32(struct usbhs_priv *priv, u32 reg, u32 data)
> +static void usbhs_write32(struct usbhs_priv *priv, u32 reg, u32 data)
>  {
>   iowrite32(data, priv->base + reg);
>  }
> --
> 2.8.1

RE: [PATCH 0/2] usb: renesas_usbhs: fix issues on specific situations

[Yoshihiro Shimoda]

Hi Felipe,

Would you review this patch set?

Best regards,
Yoshihiro Shimoda

> From: Yoshihiro Shimoda
> Sent: Wednesday, June 08, 2016 4:33 PM
> To: ba...@kernel.org
> Cc: gre...@linuxfoundation.org; linux-...@vger.kernel.org; 
> linux-renesas-soc@vger.kernel.org; sta...@vger.kernel.org;
> Yoshihiro Shimoda 
> Subject: [PATCH 0/2] usb: renesas_usbhs: fix issues on specific situations
> 
> This patch set is based on the latest Felipe's usb.git / testing/fixes branch.
> (commit id = 50c763f8c1bac0dc00f7788a75f227276c0efd54)
> 
> Yoshihiro Shimoda (2):
>   usb: renesas_usbhs: fix NULL pointer dereference in xfer_work()
>   usb: renesas_usbhs: protect the CFIFOSEL setting in usbhsg_ep_enable()
> 
>  drivers/usb/renesas_usbhs/fifo.c   | 18 ++
>  drivers/usb/renesas_usbhs/mod_gadget.c |  9 -
>  2 files changed, 22 insertions(+), 5 deletions(-)
> 
> --
> 1.9.1