Re: [PATCH v2] dmaengine: rcar-dmac: Widen DMA mask to 40 bits

[Vinod Koul]

On Mon, Feb 13, 2017 at 12:00:26PM +0100, Geert Uytterhoeven wrote:
> By default, the DMA mask covers only the low 32-bit address space, which
> causes SWIOTLB on arm64 to fall back to a bounce buffer for DMA
> transfers involving memory outside the 32-bit address space.
> 
> The R-Car DMA controller hardware supports a 40-bit address space, hence
> widen the DMA mask to 40 bits to actually make use of this feature.

Applied, thanks

-- 
~Vinod

Re: [PATCH 00/11] media: rcar-vin: fix OPS and format/pad index issues

[Niklas Söderlund]

Hi Hans,

On 2017-02-13 21:57:48 +0100, Hans Verkuil wrote:
> On 02/13/2017 06:47 PM, Niklas Söderlund wrote:
> > Hi Hans,
> > 
> > Thanks for your feedback.
> > 
> > On 2017-02-13 15:19:13 +0100, Hans Verkuil wrote:
> >> Hi Niklas,
> >>
> >> One general remark: in many commit logs you mistype 'subdeivce'. Can you
> >> fix that for the v2?
> > 
> > Will fix for v2.
> > 
> >>
> >> On 01/31/2017 04:40 PM, Niklas Söderlund wrote:
> >>> Hi,
> >>>
> >>> This series address issues with the R-Car Gen2 VIN driver. The most 
> >>> serious issue is the OPS when unbind and rebinding the i2c driver for 
> >>> the video source subdevice which have popped up as a blocker for other 
> >>> work.
> >>>
> >>> This series is broken out of my much larger R-Car Gen3 enablement series 
> >>> '[PATCHv2 00/32] rcar-vin: Add Gen3 with media controller support'. I 
> >>> plan to remove that series form patchwork and focus on these fixes first 
> >>> as they are blocking other development. Once the blocking issues are 
> >>> removed I will rebase and repost the larger Gen3 series.
> >>>
> >>> Patch 1-4 fix simple problems found while testing
> >>> 1-2 Fix format problems when the format is (re)set.
> >>> 3   Fix media pad errors
> >>> 4   Fix standard enumeration problem
> >>>
> >>> Patch 5 adds a wrapper function to retrieve the active video source 
> >>> subdevice. This is strictly not needed on Gen2 which only have one 
> >>> possible video source per VIN instance (This will change on Gen3). But 
> >>> patch 6-8,11 which fixes real issues on Gen2 make use of this wrapper, as 
> >>> not risk breaking things by removing this wrapper in this series and 
> >>> then readding it in a later Gen3 series I have chosen to keep the patch.  
> >>> Please let me know if I should drop it and rewrite patch 6-11 (if 
> >>> possible I would like to avoid that).
> >>>
> >>> Patch 6-8 deals with video source subdevice pad index handling by moving 
> >>> the information from struct rvin_dev to struct rvin_graph_entity and 
> >>> moving the pad index probing to the struct v4l2_async_notifier complete 
> >>> callback. This is needed to allow the bind/unbind fix in patch 10-11.
> >>>
> >>> Patch 9 use the pad information when calling enum_mbus_code.
> >>>
> >>> Patch 10-11 fix a OPS when unbinding/binding the video source subdevice.
> >>
> >> This will not help: you can unbind a subdev at any time, including when
> >> it is in use.
> > 
> > Yes, this series will not help remedy the problem if a device is in use.  
> > But I still find it useful since it unblocks other work, solves a OPS 
> > and if there are no current users the driver can supports unbind/bind of 
> > its subdevices, it's not perfect but it do make things a little better 
> > IMHO.
> > 
> > If it where not for me wishing to reuse the behavior introduced in patch 
> > 10-11 on R-Car Gen3 when a subdevice could not available for for other 
> > reasons than it's not bound (see bellow) I would be happy to drop the 
> > patches from this series.
> > 
> >>
> >> But why do you need this at all? You can also set suppress_bind_attrs in
> >> the adv7180 driver to prevent the bind/unbind files from appearing.
> > 
> > The primary use-case for this is on R-Car Gen3 where there are a limited 
> > number of possible routing options to connect CSI-2 devices to VIN 
> > devices (set in hardware), see table bellow. The routing possibilities 
> > are set per VIN group (VIN0-3 and VIN4-7) and not individually for each 
> > VIN device. Given this limitation some routing options will result in an 
> > N/A video source for one or more VIN devices in a VIN "group".
> > 
> >- VIN0-3 controlled by chsel register in VIN0
> >chselVIN0VIN1VIN2VIN3
> >0CSI40/VC0   CSI20/VC0   N/A CSI40/VC1
> >1CSI20/VC0   N/A CSI40/VC0   CSI20/VC1
> >2N/A CSI40/VC0   CSI20/VC0   N/A
> >3CSI40/VC0   CSI40/VC1   CSI40/VC2   CSI40/VC3
> >4CSI20/VC0   CSI20/VC1   CSI20/VC2   CSI20/VC3
> > 
> >- VIN4-7 controlled by chsel register in VIN4
> >chselVIN4VIN5VIN6VIN7
> >0CSI40/VC0   CSI20/VC0   N/A CSI40/VC1
> >1CSI20/VC0   N/A CSI40/VC0   CSI20/VC1
> >2N/A CSI40/VC0   CSI20/VC0   N/A
> >3CSI40/VC0   CSI40/VC1   CSI40/VC2   CSI40/VC3
> >4CSI20/VC0   CSI20/VC1   CSI20/VC2   CSI20/VC3
> > 
> > Example: If a VIN1 device is exposed as /dev/video1 and the current 
> > CSI-2 to VIN routing configuration controlled by the chsel register in 
> > VIN0 is set to 1 the video source routed to VIN1 is N/A. The idea then 
> > is that any open of /dev/video1 should result in -EBUSY until the CSI-2 
> > to VIN routing changes so that VIN1 is connected to a valid subdevice 
> > again. (side note: changing chsel value will not be allowed while any 
> > VIN device is opened and is done using MC)
> > 
> > This 

Re: [PATCH v4 4/4] v4l: vsp1: Remove redundant pipe->dl usage from drm

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Friday 06 Jan 2017 12:15:31 Kieran Bingham wrote:
> The pipe->dl is used only inside vsp1_du_atomic_flush(), and can be
> obtained and stored locally to simplify the code.
> 
> Signed-off-by: Kieran Bingham 

Reviewed-by: Laurent Pinchart 

As I still have issues with patch 3/4, I propose moving this one earlier in 
the series.

> ---
>  drivers/media/platform/vsp1/vsp1_drm.c  | 20 ++--
>  drivers/media/platform/vsp1/vsp1_pipe.h |  2 --
>  2 files changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> b/drivers/media/platform/vsp1/vsp1_drm.c index b4b583f7137a..d7ec980300dd
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -220,9 +220,6 @@ void vsp1_du_atomic_begin(struct device *dev)
>   struct vsp1_pipeline *pipe = >drm->pipe;
> 
>   vsp1->drm->num_inputs = pipe->num_inputs;
> -
> - /* Prepare the display list. */
> - pipe->dl = vsp1_dl_list_get(pipe->output->dlm);
>  }
>  EXPORT_SYMBOL_GPL(vsp1_du_atomic_begin);
> 
> @@ -426,10 +423,14 @@ void vsp1_du_atomic_flush(struct device *dev)
>   struct vsp1_pipeline *pipe = >drm->pipe;
>   struct vsp1_rwpf *inputs[VSP1_MAX_RPF] = { NULL, };
>   struct vsp1_entity *entity;
> + struct vsp1_dl_list *dl;
>   unsigned long flags;
>   unsigned int i;
>   int ret;
> 
> + /* Prepare the display list. */
> + dl = vsp1_dl_list_get(pipe->output->dlm);
> +
>   /* Count the number of enabled inputs and sort them by Z-order. */
>   pipe->num_inputs = 0;
> 
> @@ -484,26 +485,25 @@ void vsp1_du_atomic_flush(struct device *dev)
>   struct vsp1_rwpf *rpf = to_rwpf(>subdev);
> 
>   if (!pipe->inputs[rpf->entity.index]) {
> - vsp1_dl_list_write(pipe->dl, entity->route-
>reg,
> + vsp1_dl_list_write(dl, entity->route->reg,
>  VI6_DPR_NODE_UNUSED);
>   continue;
>   }
>   }
> 
> - vsp1_entity_route_setup(entity, pipe->dl);
> + vsp1_entity_route_setup(entity, dl);
> 
>   if (entity->ops->configure) {
> - entity->ops->configure(entity, pipe, pipe->dl,
> + entity->ops->configure(entity, pipe, dl,
>  VSP1_ENTITY_PARAMS_INIT);
> - entity->ops->configure(entity, pipe, pipe->dl,
> + entity->ops->configure(entity, pipe, dl,
>  VSP1_ENTITY_PARAMS_RUNTIME);
> - entity->ops->configure(entity, pipe, pipe->dl,
> + entity->ops->configure(entity, pipe, dl,
>  VSP1_ENTITY_PARAMS_PARTITION);
>   }
>   }
> 
> - vsp1_dl_list_commit(pipe->dl);
> - pipe->dl = NULL;
> + vsp1_dl_list_commit(dl);
> 
>   /* Start or stop the pipeline if needed. */
>   if (!vsp1->drm->num_inputs && pipe->num_inputs) {
> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h
> b/drivers/media/platform/vsp1/vsp1_pipe.h index fff122b4874d..e59bef2653f6
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_pipe.h
> +++ b/drivers/media/platform/vsp1/vsp1_pipe.h
> @@ -108,8 +108,6 @@ struct vsp1_pipeline {
> 
>   struct list_head entities;
> 
> - struct vsp1_dl_list *dl;
> -
>   unsigned int div_size;
>   unsigned int partitions;
>   struct v4l2_rect partition;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/8] v4l: vsp1: Partition phase developments

[Laurent Pinchart]

Hi Kieran,

Thank you for the patches.

On Friday 10 Feb 2017 20:27:28 Kieran Bingham wrote:
> This series presents ongoing work with the scaler partition algorithm.
> 
> It is based upon the previous partition algorithm improvements submission
> [0] This series has been pushed to a tag [1] for convenience in testing.
> 
> Patches 1-3, provide fixes and additions to the register definitions needed
> for controlling the phases of the UDS.
> 
> Patches 4 and 5 rework the partition data configuration storage, opening the
> path for Patch 6 to implement a new entity operation API. This new
> '.partition' operation gives each entity an opportunity to adapt the
> partition data based on its configuration.
> 
> A new helper function "vsp1_pipeline_propagate_partition()" is provided by
> the vsp1_pipe to walk the pipeline in reverse, with each entity having the
> opportunity to define it's input requirements to the predecessors.
> 
> Partition data is stored somewhat inefficiently in this series, whilst the
> process is established and can be considered for improvement later.
> 
> Patch 7 begins the implementation of calculating the phase values in the
> UDS, and applying them in the VI6_UDS_HPHASE register appropriately. Phase
> calculations have been established from the partition algorithm pseudo code
> provided by renesas, although the 'end phase' is always set as 0 in this
> code, it is yet to be determined if this has an effect.
> 
> Finally Patch 8, begins to allow the UDS entity to perform extra overlap at
> the partition borders to provide the filters with the required data to
> generate clean transitions from one partition to the next.

I've consolidated the two series, included all my review comments, and pushed 
the result to

git://linuxtv.org/pinchartl/media.git vsp1/partition

The result is laid out as follows.

[PATCH 01/10] v4l: vsp1: Move vsp1_video_pipeline_setup_partitions() function
[PATCH 02/10] v4l: vsp1: Calculate partition sizes at stream start
[PATCH 03/10] v4l: vsp1: Remove redundant context variables
[PATCH 04/10] v4l: vsp1: Provide UDS register updates
[PATCH 05/10] v4l: vsp1: Correct image partition parameters
[PATCH 06/10] v4l: vsp1: Move partition rectangles to struct
[PATCH 07/10] v4l: vsp1: Allow entities to participate in the partition 
algorithm
[PATCH 08/10] v4l: vsp1: Calculate UDS phase for partitions
[PATCH 09/10] v4l: vsp1: Implement left edge partition algorithm overlap
[PATCH 10/10] v4l: vsp1: Implement partition algorithm restrictions

Patches 01/10 to 06/10 look good to me, although I'm wondering whether we 
should rename vsp1_pipeline::partitions to vsp1_pipeline::num_partitions and 
vsp1_pipeline::part_table to vsp1_pipeline::partitions.

Patch 07/10 is mostly fine, I'm just a bit annoyed by the explicit pipeline 
description in struct vsp1_partition. We've discussed that previously and 
decided not to bother for now, so let's keep it as-is and revisit it when the 
rest of the series will be ready.

Patches 08/10 and 09/10 require more documentation, I don't understand what 
they do. I've rebased them on top of the review comments but haven't tested 
them yet.

Patch 10/10 needs to be reworked as the force_identity_mode mechanism doesn't 
work as explained in my review. We might also want to relax the restrictions a 
bit, based on feedback we will receive from Renesas. I'm fine merging hard 
restrictions first and relaxing them later, but at least force_identity_mode 
needs to be fixed.

You will notice that the branch I've pushed doesn't contain the suspend/resume 
fixes yet, I'll work on that next.

> [0]
> https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg08631.htm
> l [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git#vsp1/pa-p
> hases-2017-02-10
> 
> Kieran Bingham (8):
>   v4l: vsp1: Provide UDS register updates
>   v4l: vsp1: Track the SRU entity in the pipeline
>   v4l: vsp1: Correct image partition parameters
>   v4l: vsp1: Move partition rectangles to struct
>   v4l: vsp1: Operate on partition struct data directly
>   v4l: vsp1: Allow entities to participate in the partition algorithm
>   v4l: vsp1: Calculate UDS phase for partitions
>   v4l: vsp1: Implement left edge partition algorithm overlap
> 
>  drivers/media/platform/vsp1/vsp1_entity.h |   8 +-
>  drivers/media/platform/vsp1/vsp1_pipe.c   |  22 -
>  drivers/media/platform/vsp1/vsp1_pipe.h   |  49 +++-
>  drivers/media/platform/vsp1/vsp1_regs.h   |  14 ++-
>  drivers/media/platform/vsp1/vsp1_rpf.c|  40 +++---
>  drivers/media/platform/vsp1/vsp1_sru.c|  29 +-
>  drivers/media/platform/vsp1/vsp1_uds.c| 144 ++-
>  drivers/media/platform/vsp1/vsp1_video.c  |  82 -
>  drivers/media/platform/vsp1/vsp1_wpf.c|  34 +++--
>  9 files changed, 364 insertions(+), 58 deletions(-)
> 
> base-commit: 0c3b6ad6a559391f367879fd4be6d2d85625bd5a

-- 
Regards,

Laurent Pinchart

Re: [PATCH 5/8] v4l: vsp1: Operate on partition struct data directly

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Friday 10 Feb 2017 20:27:33 Kieran Bingham wrote:
> When generating the partition windows, operate directly on the partition
> struct rather than copying and duplicating the processed data
> 
> Signed-off-by: Kieran Bingham 

This looks good to me, but I'd squash it into the previous patch.

> ---
>  drivers/media/platform/vsp1/vsp1_video.c | 43 -
>  1 file changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_video.c
> b/drivers/media/platform/vsp1/vsp1_video.c index a978508a4993..5f1886bfad26
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/vsp1/vsp1_video.c
> @@ -189,17 +189,17 @@ static int __vsp1_video_try_format(struct vsp1_video
> *video, /**
>   * vsp1_video_partition - Calculate the active partition output window
>   *
> + * @partition: The active partition data
>   * @div_size: pre-determined maximum partition division size
>   * @index: partition index
>   *
> - * Returns a v4l2_rect describing the partition window.
> + * Generates the output partitioning positions.
>   */
> -static struct v4l2_rect vsp1_video_partition(struct vsp1_pipeline *pipe,
> -  unsigned int div_size,
> -  unsigned int index)
> +static void vsp1_video_partition(struct vsp1_pipeline *pipe,
> +  struct vsp1_partition *partition,
> +  unsigned int div_size, unsigned int index)
>  {
>   const struct v4l2_mbus_framefmt *format;
> - struct v4l2_rect partition;
>   unsigned int modulus;
> 
>   format = vsp1_entity_get_pad_format(>output->entity,
> @@ -208,18 +208,19 @@ static struct v4l2_rect vsp1_video_partition(struct
> vsp1_pipeline *pipe,
> 
>   /* A single partition simply processes the output size in full. */
>   if (pipe->partitions <= 1) {
> - partition.left = 0;
> - partition.top = 0;
> - partition.width = format->width;
> - partition.height = format->height;
> - return partition;
> + partition->dest.left = 0;
> + partition->dest.top = 0;
> + partition->dest.width = format->width;
> + partition->dest.height = format->height;
> +
> + return;
>   }
> 
>   /* Initialise the partition with sane starting conditions. */
> - partition.left = index * div_size;
> - partition.top = 0;
> - partition.width = div_size;
> - partition.height = format->height;
> + partition->dest.left = index * div_size;
> + partition->dest.top = 0;
> + partition->dest.width = div_size;
> + partition->dest.height = format->height;
> 
>   modulus = format->width % div_size;
> 
> @@ -242,18 +243,16 @@ static struct v4l2_rect vsp1_video_partition(struct
> vsp1_pipeline *pipe, if (modulus < div_size / 2) {
>   if (index == partitions - 1) {
>   /* Halve the penultimate partition. */
> - partition.width = div_size / 2;
> + partition->dest.width = div_size / 2;
>   } else if (index == partitions) {
>   /* Increase the final partition. */
> - partition.width = (div_size / 2) + modulus;
> - partition.left -= div_size / 2;
> + partition->dest.width = div_size / 2 + 
modulus;
> + partition->dest.left -= div_size / 2;
>   }
>   } else if (index == partitions) {
> - partition.width = modulus;
> + partition->dest.width = modulus;
>   }
>   }
> -
> - return partition;
>  }
> 
>  static void vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline
> *pipe) @@ -274,7 +273,7 @@ static void
> vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe) struct
> vsp1_partition *partition = >part_table[0];
> 
>   pipe->partitions = 1;
> - partition->dest = vsp1_video_partition(pipe, div_size, 0);
> + vsp1_video_partition(pipe, partition, div_size, 0);
> 
>   return;
>   }
> @@ -294,7 +293,7 @@ static void vsp1_video_pipeline_setup_partitions(struct
> vsp1_pipeline *pipe) for (i = 0; i < pipe->partitions; i++) {
>   struct vsp1_partition *partition = >part_table[i];
> 
> - partition->dest = vsp1_video_partition(pipe, div_size, i);
> + vsp1_video_partition(pipe, partition, div_size, i);
>   }
>  }

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/8] v4l: vsp1: Move partition rectangles to struct

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Friday 10 Feb 2017 20:27:32 Kieran Bingham wrote:
> As we develop the partition algorithm, we need to store more information
> per partition to describe the phase and other parameters.
> 
> To keep this data together, further abstract the existing v4l2_rect
> into a partition specific structure
> 
> Signed-off-by: Kieran Bingham 
> ---
>  drivers/media/platform/vsp1/vsp1_pipe.h  | 12 ++--
>  drivers/media/platform/vsp1/vsp1_rpf.c   |  4 ++--
>  drivers/media/platform/vsp1/vsp1_uds.c   |  8 +---
>  drivers/media/platform/vsp1/vsp1_video.c | 14 ++
>  drivers/media/platform/vsp1/vsp1_wpf.c   |  9 +
>  5 files changed, 32 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h
> b/drivers/media/platform/vsp1/vsp1_pipe.h index 5aa31143ce59..6494c4c75023
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_pipe.h
> +++ b/drivers/media/platform/vsp1/vsp1_pipe.h
> @@ -60,6 +60,14 @@ enum vsp1_pipeline_state {
>  };
> 
>  /*
> + * struct vsp1_partition - A description of each partition slice performed
> by HW
> + * @dest: The position and dimension of this partition in the destination
> image
> + */
> +struct vsp1_partition {
> + struct v4l2_rect dest;

Given that we only partition the image horizontally, how about just storing 
the left and width values ?

Apart from that,

Reviewed-by: Laurent Pinchart 

> +};
> +
> +/*
>   * struct vsp1_pipeline - A VSP1 hardware pipeline
>   * @pipe: the media pipeline
>   * @irqlock: protects the pipeline state
> @@ -114,8 +122,8 @@ struct vsp1_pipeline {
>   struct vsp1_dl_list *dl;
> 
>   unsigned int partitions;
> - struct v4l2_rect partition;
> - struct v4l2_rect part_table[VSP1_PIPE_MAX_PARTITIONS];
> + struct vsp1_partition *partition;
> + struct vsp1_partition part_table[VSP1_PIPE_MAX_PARTITIONS];
>  };
> 
>  void vsp1_pipeline_reset(struct vsp1_pipeline *pipe);
> diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c
> b/drivers/media/platform/vsp1/vsp1_rpf.c index b2e34a800ffa..df380a237118
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_rpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_rpf.c
> @@ -107,9 +107,9 @@ static void rpf_configure(struct vsp1_entity *entity,
>   output = vsp1_entity_get_pad_format(wpf, wpf->config,
>   RWPF_PAD_SOURCE);
> 
> - crop.width = pipe->partition.width * input_width
> + crop.width = pipe->partition->dest.width * input_width
>  / output->width;
> - crop.left += pipe->partition.left * input_width
> + crop.left += pipe->partition->dest.left * input_width
>  / output->width;
>   }
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_uds.c
> b/drivers/media/platform/vsp1/vsp1_uds.c index da8f89a31ea4..98c0836d6dcd
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_uds.c
> +++ b/drivers/media/platform/vsp1/vsp1_uds.c
> @@ -272,11 +272,13 @@ static void uds_configure(struct vsp1_entity *entity,
>   bool multitap;
> 
>   if (params == VSP1_ENTITY_PARAMS_PARTITION) {
> - const struct v4l2_rect *clip = >partition;
> + struct vsp1_partition *partition = pipe->partition;
> 
>   vsp1_uds_write(uds, dl, VI6_UDS_CLIP_SIZE,
> -(clip->width << VI6_UDS_CLIP_SIZE_HSIZE_SHIFT) 
|
> -(clip->height << 
VI6_UDS_CLIP_SIZE_VSIZE_SHIFT));
> +(partition->dest.width
> + << VI6_UDS_CLIP_SIZE_HSIZE_SHIFT) |
> +(partition->dest.height
> + << VI6_UDS_CLIP_SIZE_VSIZE_SHIFT));
>   return;
>   }
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_video.c
> b/drivers/media/platform/vsp1/vsp1_video.c index 4ade958a1c9e..a978508a4993
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/vsp1/vsp1_video.c
> @@ -271,8 +271,11 @@ static void vsp1_video_pipeline_setup_partitions(struct
> vsp1_pipeline *pipe)
> 
>   /* Gen2 hardware doesn't require image partitioning. */
>   if (vsp1->info->gen == 2) {
> + struct vsp1_partition *partition = >part_table[0];
> +
>   pipe->partitions = 1;
> - pipe->part_table[0] = vsp1_video_partition(pipe, div_size, 0);
> + partition->dest = vsp1_video_partition(pipe, div_size, 0);
> +
>   return;
>   }
> 
> @@ -288,8 +291,11 @@ static void vsp1_video_pipeline_setup_partitions(struct
> vsp1_pipeline *pipe)
> 
>   pipe->partitions = DIV_ROUND_UP(format->width, div_size);
> 
> - for (i = 0; i < pipe->partitions; i++)
> - pipe->part_table[i] = 

Re: [PATCH 1/8] v4l: vsp1: Provide UDS register updates

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Friday 10 Feb 2017 20:27:29 Kieran Bingham wrote:
> Provide register definitions required for UDS phase and partition
> algorithm support

I would mention here that those registers and bits are available on Gen3 only.

> 
> Signed-off-by: Kieran Bingham 
> ---
>  drivers/media/platform/vsp1/vsp1_regs.h | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_regs.h
> b/drivers/media/platform/vsp1/vsp1_regs.h index 47b1dee044fb..1ad819680e2f
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_regs.h
> +++ b/drivers/media/platform/vsp1/vsp1_regs.h
> @@ -388,6 +388,7 @@
>  #define VI6_UDS_CTRL_NE_RCR  (1 << 18)
>  #define VI6_UDS_CTRL_NE_GY   (1 << 17)
>  #define VI6_UDS_CTRL_NE_BCB  (1 << 16)
> +#define VI6_UDS_CTRL_AMDSLH  (1 << 2)
>  #define VI6_UDS_CTRL_TDIPC   (1 << 1)
> 
>  #define VI6_UDS_SCALE0x2304
> @@ -420,11 +421,24 @@
>  #define VI6_UDS_PASS_BWIDTH_V_MASK   (0x7f << 0)
>  #define VI6_UDS_PASS_BWIDTH_V_SHIFT  0
> 
> +#define VI6_UDS_HPHASE   0x2314
> +#define VI6_UDS_HPHASE_HSTP_MASK (0xfff << 16)
> +#define VI6_UDS_HPHASE_HSTP_SHIFT16
> +#define VI6_UDS_HPHASE_HEDP_MASK (0xfff << 0)
> +#define VI6_UDS_HPHASE_HEDP_SHIFT0
> +
>  #define VI6_UDS_IPC  0x2318
>  #define VI6_UDS_IPC_FIELD(1 << 27)
>  #define VI6_UDS_IPC_VEDP_MASK(0xfff << 0)
>  #define VI6_UDS_IPC_VEDP_SHIFT   0
> 
> +#define VI6_UDS_HSZCLIP  0x231c
> +#define VI6_UDS_HSZCLIP_HCEN (1 << 28)
> +#define VI6_UDS_HSZCLIP_HCL_OFST_MASK(0x1ff << 16)

This field spans 8 bits, not 9.

With that fixed,

Reviewed-by: Laurent Pinchart 

> +#define VI6_UDS_HSZCLIP_HCL_OFST_SHIFT   16
> +#define VI6_UDS_HSZCLIP_HCL_SIZE_MASK(0x1fff << 0)
> +#define VI6_UDS_HSZCLIP_HCL_SIZE_SHIFT   0
> +
>  #define VI6_UDS_CLIP_SIZE0x2324
>  #define VI6_UDS_CLIP_SIZE_HSIZE_MASK (0x1fff << 16)
>  #define VI6_UDS_CLIP_SIZE_HSIZE_SHIFT16

-- 
Regards,

Laurent Pinchart

Re: [PATCH 00/11] media: rcar-vin: fix OPS and format/pad index issues

[Laurent Pinchart]

Hi Hans,

On Monday 13 Feb 2017 15:19:13 Hans Verkuil wrote:
> Hi Niklas,
> 
> One general remark: in many commit logs you mistype 'subdeivce'. Can you
> fix that for the v2?
> 
> On 01/31/2017 04:40 PM, Niklas Söderlund wrote:
> > Hi,
> > 
> > This series address issues with the R-Car Gen2 VIN driver. The most
> > serious issue is the OPS when unbind and rebinding the i2c driver for
> > the video source subdevice which have popped up as a blocker for other
> > work.
> > 
> > This series is broken out of my much larger R-Car Gen3 enablement series
> > '[PATCHv2 00/32] rcar-vin: Add Gen3 with media controller support'. I
> > plan to remove that series form patchwork and focus on these fixes first
> > as they are blocking other development. Once the blocking issues are
> > removed I will rebase and repost the larger Gen3 series.
> > 
> > Patch 1-4 fix simple problems found while testing
> > 
> > 1-2 Fix format problems when the format is (re)set.
> > 3   Fix media pad errors
> > 4   Fix standard enumeration problem
> > 
> > Patch 5 adds a wrapper function to retrieve the active video source
> > subdevice. This is strictly not needed on Gen2 which only have one
> > possible video source per VIN instance (This will change on Gen3). But
> > patch 6-8,11 which fixes real issues on Gen2 make use of this wrapper, as
> > not risk breaking things by removing this wrapper in this series and
> > then readding it in a later Gen3 series I have chosen to keep the patch.
> > Please let me know if I should drop it and rewrite patch 6-11 (if
> > possible I would like to avoid that).
> > 
> > Patch 6-8 deals with video source subdevice pad index handling by moving
> > the information from struct rvin_dev to struct rvin_graph_entity and
> > moving the pad index probing to the struct v4l2_async_notifier complete
> > callback. This is needed to allow the bind/unbind fix in patch 10-11.
> > 
> > Patch 9 use the pad information when calling enum_mbus_code.
> > 
> > Patch 10-11 fix a OPS when unbinding/binding the video source subdevice.
> 
> This will not help: you can unbind a subdev at any time, including when
> it is in use.
> 
> But why do you need this at all? You can also set suppress_bind_attrs in
> the adv7180 driver to prevent the bind/unbind files from appearing.
> 
> It really makes no sense for subdevs. In fact, all subdevs should set this
> flag since in the current implementation this is completely impossible to
> implement safely.
> 
> I suggest you drop the patches relating to this and instead set the suppress
> flag.

The adv7180 is connected to an I2C controller that can be unbound. Setting the 
suppress_bind_attrs flag in the driver thus won't prevent the device from 
being unbound. suppress_bind_attrs is not a good solution for I2C drivers.

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 5/5] tests: Test suspend/resume on active pipelines

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday 01 Dec 2016 21:31:49 Kieran Bingham wrote:
> From: Kieran Bingham 
> 
> Provide a test to verify the hardware completes a functional test whilst
> performing a suspend resume cycle in parallel. Make use of the
> /sys/power/pm_test functionality provided by CONFIG_PM_DEBUG to perform
> the testing
> 
> Signed-off-by: Kieran Bingham 
> 
> ---
> v2:
> 
> - removed format iteration loop
> - modified test reporting to be once per 'suspend mode'
> - verify the pm_test mode is available, or skip
> 
>  tests/vsp-unit-test-0020.sh | 97 ++
>  1 file changed, 97 insertions(+)
>  create mode 100755 tests/vsp-unit-test-0020.sh
> 
> diff --git a/tests/vsp-unit-test-0020.sh b/tests/vsp-unit-test-0020.sh
> new file mode 100755
> index ..c9e6b79e5d06
> --- /dev/null
> +++ b/tests/vsp-unit-test-0020.sh
> @@ -0,0 +1,97 @@
> +#!/bin/sh
> +
> +#
> +# Test power-management suspend/resume whilst pipelines are active
> +#
> +# Utilise the basic RPF->WPF packing test case as a measure that the
> hardware +# is operable while we perform test suspend and resume, and
> verify that it is +# still successful even with a suspend resume cycle in
> the middle of the test. +#
> +
> +source vsp-lib.sh
> +
> +features="rpf.0 wpf.0"
> +
> +# These can be extracted from /sys/power/pm_test
> +suspend_modes="freezer devices platform processors core"
> +
> +# This extended function performs the same
> +# as it's non-extended name-sake - but runs the pipeline
> +# for 300 frames. The suspend action occurs between frame #150~#200
> +
> +test_extended_wpf_packing() {
> + pipe_configure rpf-wpf 0 0
> + format_configure rpf-wpf 0 0 ARGB32 1024x768 $format
> +
> + vsp_runner rpf.0 --count=300 &
> + vsp_runner wpf.0 --count=300 --skip=297
> +
> + local result=$(compare_frames)
> +
> + test_complete $result

This will result in test_complete being called twice. You probably want

[ x$result == xpass ] && return 0 || return 1

as in patch 4/5.

> +}
> +
> +test_hw_pipe() {
> + local format
> +
> + for format in $formats ; do

The formats variable isn't defined, I think you can just get rid of the loop 
and call test_extended_wpf_packing just once.

> + test_extended_wpf_packing RGB24
> + done
> +}
> +
> +test_suspend_resume() {
> + local result
> + local test_pid
> +
> + test_start "Testing active pipeline suspend/resume in suspend:$mode"
> +
> + # Verify the test is available
> + grep -q $mode /sys/power/pm_test
> + if [ $? != 0 ]; then
> + test_complete skip
> + return
> + fi
> +
> + # Set the hardware running in parallel while we suspend
> + test_hw_pipe &
> + test_pid=$!
> +
> + # Make sure the pipeline has time to start
> + sleep 1
> +
> + # Set the test mode
> + echo $mode > /sys/power/pm_test
> +
> + # Comence suspend

s/Comence/Commence/

> + # The pm_test framework will automatically resume after 5 seconds
> + echo mem > /sys/power/state
> +
> + # Wait for the pipeline to complete
> + wait $test_pid
> + result=$?
> +
> + if [ $result == 0 ]; then
> + test_complete pass
> + else
> + test_complete fail
> + fi
> +}
> +
> +test_main() {
> + local mode;

No need for the trailing ;

> + local suspend_test_failures

This variablee is unused.

> +
> + # Check for pm-suspend test option
> + if [ ! -e /sys/power/pm_test ] ; then
> + echo "$0: Suspend Resume testing requires CONFIG_PM_DEBUG"
> + test_complete skip

You haven't called test_start so you shouldn't call test_complete.

Some of those comments apply to patch 4/5 too.

There's no need to resubmit, I'll fix this while applying.

> + return
> + fi;
> +
> + for mode in $suspend_modes ; do
> + test_suspend_resume $mode
> + done;
> +}
> +
> +test_init $0 "$features"
> +test_run

-- 
Regards,

Laurent Pinchart

Re: [PULL REQUEST] renesas/topic/sdhi-autocmd12-resp for renesas drivers

[Ulf Hansson]

On 13 February 2017 at 11:04, Geert Uytterhoeven  wrote:
> Hi Wolfram,
>
> On Sun, Feb 12, 2017 at 12:27 PM, Wolfram Sang  wrote:
>> here is a topic branch for renesas-drivers to report back autocmd12
>> responses for SDHI. It is based on mmc/next. Please pull.
>>
>> Kind regards,
>>
>>Wolfram

Hi Wolfram,

I don't get it, why did you send this pull request to Geert and not me?

I rather pick all mmc changes via my mmc tree, to avoid conflicts etc
- unless there are some specific reasons to not. Are there?

Kind regards
Uffe

>>
>>
>> The following changes since commit 0c3630150c9af658e7375c509c670fadf052cca8:
>>
>>   mmc: core: start to break apart mmc_start_areq() (2017-02-03 10:50:23 
>> +0100)
>>
>> are available in the git repository at:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
>> renesas/topic/sdhi-autocmd12-resp
>>
>> for you to fetch changes up to 22b13c28ac6b92f3acf7a5a91575b9da004c853a:
>>
>>   mmc: host: tmio: fill in response from auto cmd12 (2017-02-07 11:47:26 
>> +0100)
>
> Thank you, scheduled for inclusion in next renesas-drivers release
> (probably renesas-drivers-2017-02-21-v4.10).
>
> BTW, it still merges cleanly into today's linux-next, despite Ulf rebasing
> his mmc/next branch almost on a daily basis.
>
> 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: [PATCHv2 2/5] scripts: Provide bin2png.sh helper

[Laurent Pinchart]

Hi Geert,

On Monday 13 Feb 2017 15:17:10 Geert Uytterhoeven wrote:
> On Mon, Feb 13, 2017 at 3:03 PM, Laurent Pinchart wrote:
> >> +PNM=$(echo $FILE | sed -e 's|\.bin$|.pnm|')
> > 
> > You can write this as
> > 
> > PNM=${FILE/.bin/.pnm}
> 
> / doesn't just match the suffix, try with waste.bin.picture.bin :-)

There's a single '.bin' string in the file names we deal with, but in general 
you're right, yes.

> PNM=${FILE%.bin}.pnm

I've used PNM={FILE/%bin/pnm}

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 3/5] logger: Log to the FTrace buffer if tracing is enabled

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday 01 Dec 2016 21:31:47 Kieran Bingham wrote:
> From: Kieran Bingham 
> 
> Extend the logger such that it will detect the tracing system, and also
> append print statement to this ring buffer.
> 
> This provides the relevant logging output interspersed in the ftrace
> logs for an effective solution to identifying the actions that caused
> the traces to occur
> 
> Signed-off-by: Kieran Bingham 

Reviewed-by: Laurent Pinchart 

> ---
>  scripts/logger.sh | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/logger.sh b/scripts/logger.sh
> index 8123f0c9f6e3..8412b0ba9a08 100755
> --- a/scripts/logger.sh
> +++ b/scripts/logger.sh
> @@ -6,6 +6,17 @@ now() {
> 
>  label=${1:+ [$1]}
> 
> +TRACE_MARKER=/sys/kernel/debug/tracing/trace_marker
> +if [ -e $TRACE_MARKER ]; then
> + extra_log_files=$TRACE_MARKER
> +fi
> +
>  while read line ; do
> - echo "$(now)$label $line"
> + newline="$(now)$label $line"
> +
> + echo "$newline"
> +
> + for f in $extra_log_files; do
> + echo "$newline" >> $f;
> + done;
>  done

-- 
Regards,

Laurent Pinchart

Re: [PATCH 00/11] media: rcar-vin: fix OPS and format/pad index issues

[Hans Verkuil]

Hi Niklas,

One general remark: in many commit logs you mistype 'subdeivce'. Can you
fix that for the v2?

On 01/31/2017 04:40 PM, Niklas Söderlund wrote:
> Hi,
> 
> This series address issues with the R-Car Gen2 VIN driver. The most 
> serious issue is the OPS when unbind and rebinding the i2c driver for 
> the video source subdevice which have popped up as a blocker for other 
> work.
> 
> This series is broken out of my much larger R-Car Gen3 enablement series 
> '[PATCHv2 00/32] rcar-vin: Add Gen3 with media controller support'. I 
> plan to remove that series form patchwork and focus on these fixes first 
> as they are blocking other development. Once the blocking issues are 
> removed I will rebase and repost the larger Gen3 series.
> 
> Patch 1-4 fix simple problems found while testing
> 1-2 Fix format problems when the format is (re)set.
> 3   Fix media pad errors
> 4   Fix standard enumeration problem
> 
> Patch 5 adds a wrapper function to retrieve the active video source 
> subdevice. This is strictly not needed on Gen2 which only have one 
> possible video source per VIN instance (This will change on Gen3). But 
> patch 6-8,11 which fixes real issues on Gen2 make use of this wrapper, as 
> not risk breaking things by removing this wrapper in this series and 
> then readding it in a later Gen3 series I have chosen to keep the patch.  
> Please let me know if I should drop it and rewrite patch 6-11 (if 
> possible I would like to avoid that).
> 
> Patch 6-8 deals with video source subdevice pad index handling by moving 
> the information from struct rvin_dev to struct rvin_graph_entity and 
> moving the pad index probing to the struct v4l2_async_notifier complete 
> callback. This is needed to allow the bind/unbind fix in patch 10-11.
> 
> Patch 9 use the pad information when calling enum_mbus_code.
> 
> Patch 10-11 fix a OPS when unbinding/binding the video source subdevice.

This will not help: you can unbind a subdev at any time, including when
it is in use.

But why do you need this at all? You can also set suppress_bind_attrs in
the adv7180 driver to prevent the bind/unbind files from appearing.

It really makes no sense for subdevs. In fact, all subdevs should set this
flag since in the current implementation this is completely impossible to
implement safely.

I suggest you drop the patches relating to this and instead set the suppress
flag.

Regards,

Hans

> 
> # echo 2-0020 > /sys/bus/i2c/drivers/adv7180/unbind
> # echo 2-0020 > /sys/bus/i2c/drivers/adv7180/bind
> 
>  adv7180 2-0020: chip found @ 0x20 (e653.i2c)
>  kobject (eaaab118): tried to init an initialized object, something is 
> seriously wrong.
>  CPU: 0 PID: 1640 Comm: bash Not tainted 4.10.0-rc4-00029-g19b80f8913cad837 #1
>  Hardware name: Generic R8A7791 (Flattened Device Tree)
>  Backtrace: 
>  [] (dump_backtrace) from [] (show_stack+0x18/0x1c)
>   r7:0016 r6:60070013 r5: r4:c0a14dd8
>  [] (show_stack) from [] (dump_stack+0x84/0xa0)
>  [] (dump_stack) from [] (kobject_init+0x3c/0x98)
>   r7:0016 r6:eaaab2e4 r5:c0a1f4dc r4:eaaab118
>  [] (kobject_init) from [] (device_initialize+0x28/0xb0)
>   r5:c0a70be8 r4:eaaab110
>  [] (device_initialize) from [] 
> (device_register+0x14/0x20)
>   r5:eaaab110 r4:eaaab110
>  [] (device_register) from [] 
> (__video_register_device+0xb38/0x11cc)
>   r5:eaaab110 r4:eaaab020
>  [] (__video_register_device) from [] 
> (rvin_v4l2_probe+0x17c/0x1e8)
>   r10: r9:eaa3c050 r8:c0a270a8 r7:eaaab3a0 r6:eaaab020 r5:c0790068
>   r4:eaaab010
>  [] (rvin_v4l2_probe) from [] 
> (rvin_digital_notify_complete+0x174/0x184)
>   r7:2006 r6:eaaab010 r5: r4:eaaab3e0
>  [] (rvin_digital_notify_complete) from [] 
> (v4l2_async_test_notify+0xe8/0xf0)
>   r7:eaaab410 r6:eaa3c050 r5:c04c6c2c r4:eaaab3e0
>  [] (v4l2_async_test_notify) from [] 
> (v4l2_async_register_subdev+0xa4/0xcc)
>   r7:eaa3c0fc r6:c0a27094 r5:eaaab3e0 r4:eaa3c050
>  [] (v4l2_async_register_subdev) from [] 
> (adv7180_probe+0x350/0x3e0)
>   r9:eaa3c050 r8: r7: r6: r5:eb2cbe00 r4:eaa3c010
>  [] (adv7180_probe) from [] (i2c_device_probe+0x238/0x250)
>   r9:000e r8:c0a264dc r7:eb2cbe20 r6:c0a264dc r5:c04973f0 r4:eb2cbe00
>  [] (i2c_device_probe) from [] 
> (driver_probe_device+0x1f8/0x2c0)
>   r9:000e r8:c0a264dc r7: r6:c0a70c18 r5:c0a70c0c r4:eb2cbe20
>  [] (driver_probe_device) from [] (bind_store+0x94/0xe8)
>   r10: r9:0051 r8:0007 r7:c0a26058 r6:eb2cbe54 r5:c0a264dc
>   r4:eb2cbe20 r3:ea60b000
>  [] (bind_store) from [] (drv_attr_store+0x2c/0x38)
>   r9:0051 r8:eb2daa0c r7:ea58ff80 r6:eb2daa00 r5:ea87a4c0 r4:c03bbc3c
>  [] (drv_attr_store) from [] (sysfs_kf_write+0x40/0x4c)
>   r5:ea87a4c0 r4:c03bb6e4
>  [] (sysfs_kf_write) from [] 
> (kernfs_fop_write+0x13c/0x1ac)
>   r5:ea87a4c0 r4:0007
>  [] (kernfs_fop_write) from [] (__vfs_write+0x34/0x114)
>   r9:ea58e000 r8: r7:0007 r6:ea58ff80 r5:ea52a480 r4:c023db14
>  

Re: [PATCHv2 2/5] scripts: Provide bin2png.sh helper

[Geert Uytterhoeven]

Hi Laurent,

On Mon, Feb 13, 2017 at 3:03 PM, Laurent Pinchart
 wrote:
>> +PNM=$(echo $FILE | sed -e 's|\.bin$|.pnm|')
>
> You can write this as
>
> PNM=${FILE/.bin/.pnm}

/ doesn't just match the suffix, try with waste.bin.picture.bin :-)

PNM=${FILE%.bin}.pnm

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] scripts: bin2png: Support conversion of all files in a directory

[Laurent Pinchart]

When the first argument is a directory name instead of a file name,
convert all test script output binaries in that directory.

Signed-off-by: Laurent Pinchart 
---
 scripts/bin2png.sh | 41 -
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/scripts/bin2png.sh b/scripts/bin2png.sh
index 2563e87d4187..90fdfb077715 100755
--- a/scripts/bin2png.sh
+++ b/scripts/bin2png.sh
@@ -1,25 +1,40 @@
 #!/bin/sh
 
-FILE="$1"
+FILE=${1:-.}
 
-PNM=${FILE/.bin/.pnm}
-PNG=${FILE/.bin/.png}
+convert_image() {
+   local file=$1
+   local pnm=${file/%bin/pnm}
+   local png=${file/%bin/png}
 
-fmt=$(echo $FILE | sed -e 's|.*-\([[:alnum:]]*\)-\([0-9]*x[0-9]*\).*.bin|\1|' 
| tr '[:lower:]' '[:upper:]')
-size=$(echo $FILE | sed -e 's|.*-\([[:alnum:]]*\)-\([0-9]*x[0-9]*\).*.bin|\2|')
+   local format=$(echo $file | sed -e 
's|.*-\([[:alnum:]]*\)-\([0-9]*x[0-9]*\).*.bin|\1|' | tr '[:lower:]' 
'[:upper:]')
+   local size=$(echo $file   | sed -e 
's|.*-\([[:alnum:]]*\)-\([0-9]*x[0-9]*\).*.bin|\2|')
 
-case $fmt in
+   case $format in
yuv*|yvu*)
-   fmt=$(echo $fmt | tr 'M' 'P')
+   format=$(echo $format | tr 'M' 'P')
;;
nv*)
-   fmt=$(echo $fmt | tr -d 'M')
+   format=$(echo $format | tr -d 'M')
;;
*rgb*)
-   fmt=$(echo $fmt | tr -d 'AX')
+   format=$(echo $format | tr -d 'AX')
;;
-esac
+   esac
 
-raw2rgbpnm -s $size -f $fmt $FILE $PNM && \
-   convert $PNM $PNG
-rm $PNM
+   raw2rgbpnm -f $format -s $size $file $pnm && \
+   convert $pnm $png
+   rm $pnm
+}
+
+if [ -d $FILE ] ; then
+   if [ $(ls $FILE/vsp-unit-test-00*-*frame*.bin 2>/dev/null | wc -l) != 0 
] ; then
+   for f in $FILE/vsp-unit-test-00*-*frame*.bin ; do
+   convert_image $f
+   done
+   fi
+elif [ -f $FILE ] ; then
+   convert_image $FILE
+else
+   echo "Usage: $0 "
+fi
-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] mmc: host: tmio: fix minor typos in a comment

[Wolfram Sang]

> > -/* Definitions for values the CTRL_STATUS register can take. */
> > +/* Definitions for values the CTL_STATUS register can take */
> >  #define TMIO_STAT_CMDRESPENDBIT(0)
> >  #define TMIO_STAT_DATAEND   BIT(2)
> >  #define TMIO_STAT_CARD_REMOVE   BIT(3)
> 
> Is a similar update for CTRL_STATUS2 appropriate a few lines further down?

Yes, and SDIO_STATUS as well. Will fix and resend. Thanks!



signature.asc
Description: PGP signature

Re: [PATCHv2 2/5] scripts: Provide bin2png.sh helper

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday 01 Dec 2016 21:31:46 Kieran Bingham wrote:
> From: Kieran Bingham 
> 
> Identify the size and format from the test output filename, and pass
> to raw2rgbpnm for conversion to a PNM file.
> 
> From there we can convert easily to a PNG output file.
> 
> Signed-off-by: Kieran Bingham 
> 
> ---
> v2:
> 
> - use 'convert' to proces png files to png
> - strip '.bin' from target filenames
> 
>  scripts/Makefile   |  2 +-
>  scripts/bin2png.sh | 36 
>  2 files changed, 37 insertions(+), 1 deletion(-)
>  create mode 100755 scripts/bin2png.sh
> 
> diff --git a/scripts/Makefile b/scripts/Makefile
> index 8c452f4c54ce..6586b2989aed 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -1,4 +1,4 @@
> -SCRIPTS=logger.sh vsp-lib.sh
> +SCRIPTS=$(wildcard *.sh)
> 
>  all:
> 
> diff --git a/scripts/bin2png.sh b/scripts/bin2png.sh
> new file mode 100755
> index ..bde1ddfa3eab
> --- /dev/null
> +++ b/scripts/bin2png.sh
> @@ -0,0 +1,36 @@
> +#!/bin/sh
> +
> +FILE="$1"
> +
> +PNM=$(echo $FILE | sed -e 's|\.bin$|.pnm|')

You can write this as

PNM=${FILE/.bin/.pnm}

> +PNG=$(echo $FILE | sed -e 's|\.bin$|.png|')

Ditto.

> +fmt=$(echo $FILE | sed -e
> 's|.*-\([[:alnum:]]*\)-\([0-9]*x[0-9]*\).*.bin|\1|') +size=$(echo $FILE |
> sed -e 's|.*-\([[:alnum:]]*\)-\([0-9]*x[0-9]*\).*.bin|\2|') +
> +case $fmt in
> + yuv410m|yvu410m|yuv411m|yuv420m|yvu420m|yuv422m|yvu422m|yuv444m|
yvu444m)
> + fmt=`echo $fmt | tr '[:lower:]' '[:upper:]'`
> + fmt=`echo $fmt | tr 'M' 'P'`
> + ;;
> + nv12m|nv21m|nv16m|nv61m)
> + fmt=`echo $fmt | tr '[:lower:]' '[:upper:]'`
> + fmt=`echo $fmt | tr -d 'M'`
> + ;;
> + argb555|xrgb555)
> + fmt=RGB555X

raw2rgbpnm doesn't support RGB555X, I think you should use RGB555.

> + ;;
> + argb32|xrgb32)
> + fmt=RGB32
> + ;;
> + abgr32|xbgr32)
> + fmt=BGR32
> + ;;

You could group all those cases by just removing the leading A or X.

No need to resubmit, I'll fix while applying.

> + *)
> + fmt=`echo $fmt | tr '[:lower:]' '[:upper:]'`
> + ;;
> +esac
> +
> +raw2rgbpnm -s $size -f $fmt $FILE $PNM && \
> + convert $PNM $PNG
> +rm $PNM

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 1/5] scripts: Test suite runner

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday 01 Dec 2016 21:31:45 Kieran Bingham wrote:
> From: Kieran Bingham 
> 
> Provide a utility script to execute all vsp unit tests, as well
> as the option to execute multiple iterations of the suite.
> 
> Signed-off-by: Kieran Bingham 
> 
> ---
> v2
> - remove spurious uses of ';'
> - fix output logging
> 
>  scripts/vsp-tests.sh | 49 +
> 1 file changed, 49 insertions(+)
>  create mode 100755 scripts/vsp-tests.sh
> 
> diff --git a/scripts/vsp-tests.sh b/scripts/vsp-tests.sh
> new file mode 100755
> index ..8b21176ccc09
> --- /dev/null
> +++ b/scripts/vsp-tests.sh
> @@ -0,0 +1,49 @@
> +#!/bin/sh
> +
> +##
> +## VSP Tests runner
> +##
> +## Automatically execute all vsp-unit tests
> +## Move test failure results to a specific folder for
> +## the running kernel version
> +##
> +## An argument can be provided to specify the number of
> +## iterations to perform
> +##
> +## usage:
> +##  ./vsp-tests.sh 
> +##
> +##   n: Number of iterations to execute test suite
> +##
> +
> +KERNEL_VERSION=`uname -r`
> +
> +run_test() {
> + echo $1

I would print "- $1" to make the output easier to read.

> + ./$1
> +
> + if [ $(ls *.bin 2>/dev/null | wc -l) != 0 ]
> + then
> + RESULTS_DIR=$KERNEL_VERSION/test-$1/$2/

Let's make local variables local.

> +
> + echo "Moving *.bin to $RESULTS_DIR"

I would remove this message, it doesn't add much value.

> + mkdir -p $RESULTS_DIR
> + mv *.bin $RESULTS_DIR
> + for f in $RESULTS_DIR/*.bin
> + do
> + ./bin2png.sh "$f"
> + done

Converting binary files to png on the target would slow down the tests. How 
about performing the conversion on the host instead ?

> + fi
> +}
> +
> +run_suite() {
> + echo "Test loop $1"

How about "--- Test loop $1 ---" here for the same reason as above ?

No need to resubmit, I'll fix while applying.

> +
> + for test in vsp-unit-test*.sh; do
> + run_test $test $1;
> + done;
> +}
> +
> +for loop in `seq 1 1 $1`; do
> + run_suite $loop
> +done;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 4/4] arm64: dts: r8a7796: salvator-x: add SCIF1 (DEBUG1)

[Geert Uytterhoeven]

Hi Simon,

On Mon, Feb 13, 2017 at 2:05 PM, Simon Horman  wrote:
> On Wed, Feb 08, 2017 at 12:19:55PM +0100, Ulrich Hecht wrote:
>> On Wed, Feb 8, 2017 at 11:54 AM, Simon Horman  wrote:
>> > On Fri, Feb 03, 2017 at 11:38:20AM +0100, Ulrich Hecht wrote:
>> >> Enables the SCIF hooked up to the DEBUG1 connector.
>> >>
>> >> Signed-off-by: Ulrich Hecht 
>> >> Reviewed-by: Geert Uytterhoeven 
>> >
>> > Hi Ulrich,
>> >
>> > could you clarify the dependency of this patch on earlier ones in the
>> > series. Is it safe to queue this up independently of the other patches?
>>
>> It does not actually depend on the other patches. I think I included
>> it here because I used that port for testing. Should be safe.
>
> Thanks but I dont' seem to have scif1 in r8a7796.dtsi.
> Should this patch be updated to use hscif1 which is (now) present in
> r8a7796.dtsi?

Nah, you just need

[PATCH v2 2/3] arm64: renesas: r8a7796: Add all SCIF nodes

first :-)

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 v4 4/4] arm64: dts: r8a7796: salvator-x: add SCIF1 (DEBUG1)

[Simon Horman]

On Wed, Feb 08, 2017 at 12:19:55PM +0100, Ulrich Hecht wrote:
> On Wed, Feb 8, 2017 at 11:54 AM, Simon Horman  wrote:
> > On Fri, Feb 03, 2017 at 11:38:20AM +0100, Ulrich Hecht wrote:
> >> Enables the SCIF hooked up to the DEBUG1 connector.
> >>
> >> Signed-off-by: Ulrich Hecht 
> >> Reviewed-by: Geert Uytterhoeven 
> >
> > Hi Ulrich,
> >
> > could you clarify the dependency of this patch on earlier ones in the
> > series. Is it safe to queue this up independently of the other patches?
> 
> It does not actually depend on the other patches. I think I included
> it here because I used that port for testing. Should be safe.

Thanks but I dont' seem to have scif1 in r8a7796.dtsi.
Should this patch be updated to use hscif1 which is (now) present in
r8a7796.dtsi?

Re: [PATCH v2 3/4] arm64: renesas: r8a7796 dtsi: Add all HSCIF nodes

[Simon Horman]

On Mon, Feb 13, 2017 at 03:06:21PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 2/13/2017 3:01 PM, Simon Horman wrote:
> 
> >>>Add the device nodes for all HSCIF serial ports, incl. clocks, and
> >>>clock domain.
> >>
> >>   Not power domain?
> >>
> >>>Signed-off-by: Ulrich Hecht 
> >>>Reviewed-by: Geert Uytterhoeven 
> >>>---
> >>>arch/arm64/boot/dts/renesas/r8a7796.dtsi | 70 
> >>>
> >>>1 file changed, 70 insertions(+)
> >>>
> >>>diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi 
> >>>b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> >>>index 7bf0f2f..c508e0f 100644
> >>>--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> >>>+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> >>>@@ -421,6 +421,76 @@
> >>>   };
> >>>   };
> >>>
> >>>+  hscif0: serial@e654 {
> >>>+  compatible = "renesas,hscif-r8a7796",
> >>>+   "renesas,rcar-gen3-hscif",
> >>>+   "renesas,hscif";
> >>>+  reg = <0 0xe654 0 96>;
> >>
> >>   0xc0?
> >
> >Do you mean 96 should be 0xc0 (= 192 = 2 * 96) ?
> 
>Oops! 0x60, of course.

Thanks,

I queued up this patch after
- s/clock domain/power domain/ in changelog
- Updating to use 0x60 as you suggest above.

> >If so, perhaps the r8a7795 needs updating too?

Thanks, I'll see about sending an update.

Re: [PATCH 4/4] mmc: host: tmio: fill in response from auto cmd12

[Simon Horman]

On Sun, Feb 12, 2017 at 12:12:27PM +0100, Wolfram Sang wrote:
> After we received the dataend interrupt, R1 response register carries
> the value from the automatically generated stop command. Report that
> info back to the MMC block layer, so we will be notified in case of e.g.
> ECC errors which happened during the last transfer.
> 
> Signed-off-by: Wolfram Sang 

Reviewed-by: Simon Horman 

Re: [PATCH v3 0/7] Add V4L2 SDR (DRIF & MAX2175) driver

[Hans Verkuil]

On 02/07/2017 04:02 PM, Ramesh Shanmugasundaram wrote:
> Hi Media, DT maintainers, All,
> 
> This patch set contains two drivers
>  - Renesas R-Car Digital Radio Interface (DRIF) driver
>  - Maxim's MAX2175 RF to Bits tuner driver
> 
> These patches were based on top of media-tree repo
> commit: 47b037a0512d9f8675ec2693bed46c8ea6a884ab
> 
> These two drivers combined together expose a V4L2 SDR device that is 
> compliant with the V4L2 framework [1]. Agreed review comments are 
> incorporated in this series.
> 
> The rcar_drif device is modelled using "renesas,bonding" property. The 
> discussion on this property is available here [2].

Other than the single comment I had it all looks good. Once I have Acks for the
bindings and from Laurent for the rcar part I can merge it.

Regards,

Hans

> 
> Change history:
> 
> v2 -> v3:
> rcar_drif:
>  - Reduced DRIF DT properties to expose tested I2S mode only (Hans - 
> discussion on #v4l)
>  - Fixed error path clean up of ctrl_hdl on rcar_drif
> 
> v1 -> v2:
>  - SDR formats renamed as "planar" instead of sliced (Hans)
>  - Documentation formatting correction (Laurent)
> 
>  rcar_drif:
>  - DT model using "bonding" property
>  - Addressed Laurent's coments on bindings - DT optional parameters rename & 
> rework
>  - Addressed Han's comments on driver
>  - Addressed Geert's comments on DT
> 
>  max2175:
>  - Avoided scaling using method proposed by Antti. Thanks
>  - Bindings is a separate patch (Rob)
>  - Addressed Rob's comment on bindings
>  - Added Custom controls documentation (Laurent)
> 
> [1] v4l2-compliance report
> 
> root@salvator-x:~# v4l2-compliance -S /dev/swradio0
> v4l2-compliance SHA   : 788b674f3827607c09c31be11c91638f816aa6ae
> 
> Driver Info:
> Driver name   : rcar_drif
> Card type : R-Car DRIF
> Bus info  : platform:R-Car DRIF
> Driver version: 4.10.0
> Capabilities  : 0x8531
> SDR Capture
> Tuner
> Read/Write
> Streaming
> Extended Pix Format
> Device Capabilities
> Device Caps   : 0x0531
> SDR Capture
> Tuner
> Read/Write
> Streaming
> Extended Pix Format
> 
> Compliance test for device /dev/swradio0 (not using libv4l2):
> 
> Required ioctls:
> test VIDIOC_QUERYCAP: OK
> 
> Allow for multiple opens:
> test second sdr open: OK
> test VIDIOC_QUERYCAP: OK
> test VIDIOC_G/S_PRIORITY: OK
> test for unlimited opens: OK
> 
> Debug ioctls:
> test VIDIOC_DBG_G/S_REGISTER: OK
> test VIDIOC_LOG_STATUS: OK
> 
> Input ioctls:
> test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK
> test VIDIOC_G/S_FREQUENCY: OK
> test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
> test VIDIOC_ENUMAUDIO: OK (Not Supported)
> test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
> test VIDIOC_G/S_AUDIO: OK (Not Supported)
> Inputs: 0 Audio Inputs: 0 Tuners: 1
> 
> Output ioctls:
> test VIDIOC_G/S_MODULATOR: OK (Not Supported)
> test VIDIOC_G/S_FREQUENCY: OK
> test VIDIOC_ENUMAUDOUT: OK (Not Supported)
> test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
> test VIDIOC_G/S_AUDOUT: OK (Not Supported)
> Outputs: 0 Audio Outputs: 0 Modulators: 0
> 
> Input/Output configuration ioctls:
> test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
> test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
> test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
> test VIDIOC_G/S_EDID: OK (Not Supported)
> 
> Control ioctls:
> test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
> test VIDIOC_QUERYCTRL: OK
> test VIDIOC_G/S_CTRL: OK
> test VIDIOC_G/S/TRY_EXT_CTRLS: OK
> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
> test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
> Standard Controls: 5 Private Controls: 3
> 
> Format ioctls:
> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
> test VIDIOC_G/S_PARM: OK (Not Supported)
> test VIDIOC_G_FBUF: OK (Not Supported)
> test VIDIOC_G_FMT: OK
> test VIDIOC_TRY_FMT: OK
> test VIDIOC_S_FMT: OK
> test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
> test Cropping: OK (Not Supported)
> test Composing: OK (Not Supported)
> test Scaling: OK (Not Supported)
> 
> Codec ioctls:
> test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
> test VIDIOC_G_ENC_INDEX: OK (Not Supported)
> test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
> 
> Buffer ioctls:
> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
> test 

Re: [PATCH 3/4] mmc: host: tmio: don't BUG on unsupported stop commands

[Simon Horman]

On Sun, Feb 12, 2017 at 12:12:26PM +0100, Wolfram Sang wrote:
> Halting the kernel on an unsupported stop command seems overkill, report
> the error and say what we already did (due to autocmd12) instead.
> 
> Signed-off-by: Wolfram Sang 

Reviewed-by: Simon Horman 

Re: [PATCH v3 3/7] media: i2c: max2175: Add MAX2175 support

[Hans Verkuil]

Hi Ramesh,

A few little nitpicks:

On 02/07/2017 04:02 PM, Ramesh Shanmugasundaram wrote:
> This patch adds driver support for the MAX2175 chip. This is Maxim
> Integrated's RF to Bits tuner front end chip designed for software-defined
> radio solutions. This driver exposes the tuner as a sub-device instance
> with standard and custom controls to configure the device.
> 
> Signed-off-by: Ramesh Shanmugasundaram 
> 
> ---
>  Documentation/media/v4l-drivers/index.rst   |1 +
>  Documentation/media/v4l-drivers/max2175.rst |   60 ++
>  drivers/media/i2c/Kconfig   |4 +
>  drivers/media/i2c/Makefile  |2 +
>  drivers/media/i2c/max2175/Kconfig   |8 +
>  drivers/media/i2c/max2175/Makefile  |4 +
>  drivers/media/i2c/max2175/max2175.c | 1438 
> +++
>  drivers/media/i2c/max2175/max2175.h |  108 ++
>  8 files changed, 1625 insertions(+)
>  create mode 100644 Documentation/media/v4l-drivers/max2175.rst
>  create mode 100644 drivers/media/i2c/max2175/Kconfig
>  create mode 100644 drivers/media/i2c/max2175/Makefile
>  create mode 100644 drivers/media/i2c/max2175/max2175.c
>  create mode 100644 drivers/media/i2c/max2175/max2175.h
> 



> +
> +static const struct v4l2_ctrl_config max2175_i2s_en = {
> + .ops = _ctrl_ops,
> + .id = V4L2_CID_MAX2175_I2S_ENABLE,
> + .name = "I2S Enable",
> + .type = V4L2_CTRL_TYPE_BOOLEAN,
> + .min = 0,
> + .max = 1,
> + .step = 1,
> + .def = 1,
> +};
> +
> +/*
> + * HSLS value control LO freq adjacent location configuration.
> + * Refer to Documentation/media/v4l-drivers/max2175 for more details.
> + */
> +static const struct v4l2_ctrl_config max2175_hsls = {
> + .ops = _ctrl_ops,
> + .id = V4L2_CID_MAX2175_HSLS,
> + .name = "HSLS above/below desired",

The convention is that the descriptions of controls follow the English 'title' 
rules
w.r.t. caps. See v4l2-ctrls.c.

This would become: "HSLS Above/Below Desired".

> + .type = V4L2_CTRL_TYPE_INTEGER,

Shouldn't this be a boolean control?

> + .min = 0,
> + .max = 1,
> + .step = 1,
> + .def = 1,
> +};
> +
> +/*
> + * Rx modes below are a set of preset configurations that decides the tuner's
> + * sck and sample rate of transmission. They are separate for EU & NA 
> regions.
> + * Refer to Documentation/media/v4l-drivers/max2175 for more details.
> + */
> +static const char * const max2175_ctrl_eu_rx_modes[] = {
> + [MAX2175_EU_FM_1_2] = "EU FM 1.2",
> + [MAX2175_DAB_1_2]   = "DAB 1.2",
> +};
> +
> +static const char * const max2175_ctrl_na_rx_modes[] = {
> + [MAX2175_NA_FM_1_0] = "NA FM 1.0",
> + [MAX2175_NA_FM_2_0] = "NA FM 2.0",
> +};
> +
> +static const struct v4l2_ctrl_config max2175_eu_rx_mode = {
> + .ops = _ctrl_ops,
> + .id = V4L2_CID_MAX2175_RX_MODE,
> + .name = "RX MODE",

MODE -> Mode.

> + .type = V4L2_CTRL_TYPE_MENU,
> + .max = ARRAY_SIZE(max2175_ctrl_eu_rx_modes) - 1,
> + .def = 0,
> + .qmenu = max2175_ctrl_eu_rx_modes,
> +};
> +
> +static const struct v4l2_ctrl_config max2175_na_rx_mode = {
> + .ops = _ctrl_ops,
> + .id = V4L2_CID_MAX2175_RX_MODE,
> + .name = "RX MODE",

Ditto.

> + .type = V4L2_CTRL_TYPE_MENU,
> + .max = ARRAY_SIZE(max2175_ctrl_na_rx_modes) - 1,
> + .def = 0,
> + .qmenu = max2175_ctrl_na_rx_modes,
> +};
> +

Regards,

Hans

Re: [PATCH 2/4] mmc: host: tmio: fix minor typos in a comment

[Simon Horman]

On Sun, Feb 12, 2017 at 12:12:25PM +0100, Wolfram Sang wrote:
> Making sure we match the actual register name.
> 
> Signed-off-by: Wolfram Sang 
> ---
>  drivers/mmc/host/tmio_mmc.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index b20b451ad90daa..8a4e99ffe64eb1 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -54,7 +54,7 @@
>  #define TMIO_STOP_STPBIT(0)
>  #define TMIO_STOP_SECBIT(8)
>  
> -/* Definitions for values the CTRL_STATUS register can take. */
> +/* Definitions for values the CTL_STATUS register can take */
>  #define TMIO_STAT_CMDRESPENDBIT(0)
>  #define TMIO_STAT_DATAEND   BIT(2)
>  #define TMIO_STAT_CARD_REMOVE   BIT(3)

Is a similar update for CTRL_STATUS2 appropriate a few lines further down?

Re: [PATCH/RESEND] arm64: defconfig: Enlarge CMA alignment to 2 MiB

[Will Deacon]

On Mon, Feb 13, 2017 at 12:04:18PM +0100, Geert Uytterhoeven wrote:
> Some IOMMUs (e.g. Renesas IPMMU/VMSA) support only page sizes of 4 KiB,
> 2 MiB, and 1 GiB.
> 
> With the default setting of CONFIG_CMA_ALIGNMENT = 8, allocations larger
> than 1 MiB are aligned to a 1 MiB boundary only.  Hence a 2 MiB
> allocation may not be aligned, leading to a mapping of 512 4 KiB pages.
> 
> Increase CONFIG_CMA_ALIGNMENT to allow mapping a 2 MiB buffer using a
> single PTE, decreasing memory usage and TLB pressure.
> 
> Signed-off-by: Geert Uytterhoeven 
> ---
> Is this useful?

I assume you're proposing it because you see an improvement? :)

> Should there instead be different defaults in Kconfig, depending on
> enabled platform support?

I don't object to updating defconfig as a quick hack, but the right solution
is probably to make the core Kconfig default value overridable by the
architecture.

Will

Re: [PATCH 1/4] mmc: host: tmio: use defines for CTL_STOP_INTERNAL_ACTION values

[Simon Horman]

On Sun, Feb 12, 2017 at 12:12:24PM +0100, Wolfram Sang wrote:
> Don't use hardcoded values.
> 
> Signed-off-by: Wolfram Sang 

Reviewed-by: Simon Horman 

Re: [PATCH/RFC v3 net] ravb: unmap descriptors when freeing rings

[Simon Horman]

On Fri, Feb 10, 2017 at 02:52:50PM +0100, Geert Uytterhoeven wrote:
> On Wed, Jan 25, 2017 at 5:18 PM, Sergei Shtylyov
>  wrote:
> > On 01/24/2017 09:21 PM, Simon Horman wrote:
> >
> >> From: Kazuya Mizuguchi 
> >>
> >> "swiotlb buffer is full" errors occur after repeated initialisation of a
> >> device - f.e. suspend/resume or ip link set up/down. This is because
> >> memory
> >> mapped using dma_map_single() in ravb_ring_format() and ravb_start_xmit()
> >> is not released.  Resolve this problem by unmapping descriptors when
> >> freeing rings.
> >
> >
> >Could you look into the sh_eth driver which seems to have the same issue?
> 
> Indeed, after a few suspend/resume cycles on r8a7791/koelsch:
> 
> WARNING: CPU: 1 PID: 1699 at lib/dma-debug.c:517 add_dma_entry+0xfc/0x148
> DMA-API: exceeded 7 overlapping mappings of cacheline 0x01a827e3

Thanks for confirming that. It matches my expectation after
reading of the sh_eth code.

Re: [PATCH v2 3/4] arm64: renesas: r8a7796 dtsi: Add all HSCIF nodes

[Sergei Shtylyov]

Hello!

On 2/13/2017 3:01 PM, Simon Horman wrote:


Add the device nodes for all HSCIF serial ports, incl. clocks, and
clock domain.


   Not power domain?


Signed-off-by: Ulrich Hecht 
Reviewed-by: Geert Uytterhoeven 
---
arch/arm64/boot/dts/renesas/r8a7796.dtsi | 70 
1 file changed, 70 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi 
b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 7bf0f2f..c508e0f 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -421,6 +421,76 @@
};
};

+   hscif0: serial@e654 {
+   compatible = "renesas,hscif-r8a7796",
+"renesas,rcar-gen3-hscif",
+"renesas,hscif";
+   reg = <0 0xe654 0 96>;


   0xc0?


Do you mean 96 should be 0xc0 (= 192 = 2 * 96) ?


   Oops! 0x60, of course.


If so, perhaps the r8a7795 needs updating too?


   Probably.

MBR, Sergei

Re: [PATCH v2 3/4] arm64: renesas: r8a7796 dtsi: Add all HSCIF nodes

[Simon Horman]

On Thu, Dec 08, 2016 at 12:09:02PM +0300, Sergei Shtylyov wrote:
> On 12/7/2016 7:44 PM, Ulrich Hecht wrote:
> 
> >Add the device nodes for all HSCIF serial ports, incl. clocks, and
> >clock domain.
> 
>Not power domain?
> 
> >Signed-off-by: Ulrich Hecht 
> >Reviewed-by: Geert Uytterhoeven 
> >---
> > arch/arm64/boot/dts/renesas/r8a7796.dtsi | 70 
> > 
> > 1 file changed, 70 insertions(+)
> >
> >diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi 
> >b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> >index 7bf0f2f..c508e0f 100644
> >--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> >+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> >@@ -421,6 +421,76 @@
> > };
> > };
> >
> >+hscif0: serial@e654 {
> >+compatible = "renesas,hscif-r8a7796",
> >+ "renesas,rcar-gen3-hscif",
> >+ "renesas,hscif";
> >+reg = <0 0xe654 0 96>;
> 
>0xc0?

Do you mean 96 should be 0xc0 (= 192 = 2 * 96) ?

If so, perhaps the r8a7795 needs updating too?

RE: [PATCH] ARM: shmobile: r7s72100: add restart handler

[Chris Brandt]

Hi Geert,

On Monday, February 13, 2017, Geert Uytterhoeven wrote:
> On Fri, Feb 10, 2017 at 8:46 PM, Chris Brandt 
> wrote:
> > On Friday, February 10, 2017, Geert Uytterhoeven wrote:
> >> Alternatively, you can write a restart driver (cfr.
> >> drivers/power/reset/rmobile-reset.c) that binds against a
> >> "renesas,r7s72100-wdt" device node, but doesn't implement watchdog
> >> functionality.
> >> You're gonna need DT bindings anyway.
> >
> > I like that idea. That should take me no time at all.
> > Thank you.
> >
> > Do you think I can still keep my 'weak function' idea in there??
> >
> >
> > extern void __attribute__ ((weak)) prepare_for_restart(void) {
> > /* override to do board specific stuff */ }
> >
> > static int renesas_wdt_reset_handler(struct notifier_block *this,
> >  unsigned long mode, void *cmd) {
> > pr_debug("%s %lu\n", __func__, mode);
> >
> > prepare_for_restart();
> >
> > /* set WDT for reset */
> > . . .
> >
> > return NOTIFY_DONE;
> > }
> 
> What do you want to use the board-specific function for?
> Have a board-specific reset handler, or do some preparatory cleanup?
> 
> In case of the former, please write a separate driver that registers  a
> reset handler with a higher priority.
> In case of the latter, please use register_reboot_notifier() in the driver
> that needs it.

On my board (the RSK), I don't really care. I was thinking more about
other users boards. In other words, what should I tell them what they
should do?
I will suggest your recommendations above (not include the weak modifier).


> > Or...do you think I can just use the rmobile-reset.c driver and just
> > add WDT to it?
> >
> > Honestly, the only thing different will be rmobile_reset_handler().
> > I could make a rmobile_wdt_reset_handler() and I could just pass in a
> > different notifier_block depending on the DT.
> >
> > What do you think?
> 
> Given the small amount of code to add to the existing driver, and the
> sheer amount of boilerplate for writing a new driver, I'm inclined to say
> yes.
> But in the end, it's up to the subsystem maintainer to decide.
> 
> > static const struct of_device_id rmobile_reset_of_match[] = {
> > { .compatible = "renesas,sysc-rmobile", },
> > { .compatible = "renesas,wdt-rmobile", },
> 
> renesas,r7s72100-wdt

OK. Thanks!


Chris

[PATCH/RESEND] arm64: defconfig: Enlarge CMA alignment to 2 MiB

[Geert Uytterhoeven]

Some IOMMUs (e.g. Renesas IPMMU/VMSA) support only page sizes of 4 KiB,
2 MiB, and 1 GiB.

With the default setting of CONFIG_CMA_ALIGNMENT = 8, allocations larger
than 1 MiB are aligned to a 1 MiB boundary only.  Hence a 2 MiB
allocation may not be aligned, leading to a mapping of 512 4 KiB pages.

Increase CONFIG_CMA_ALIGNMENT to allow mapping a 2 MiB buffer using a
single PTE, decreasing memory usage and TLB pressure.

Signed-off-by: Geert Uytterhoeven 
---
Is this useful?

Should there instead be different defaults in Kconfig, depending on
enabled platform support?
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 7735cfba01c8e650..7af449f3f41435bc 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -144,6 +144,7 @@ CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
 CONFIG_DMA_CMA=y
+CONFIG_CMA_ALIGNMENT=9
 CONFIG_MTD=y
 CONFIG_MTD_BLOCK=y
 CONFIG_MTD_M25P80=y
-- 
1.9.1

[PATCH v2] dmaengine: rcar-dmac: Widen DMA mask to 40 bits

[Geert Uytterhoeven]

By default, the DMA mask covers only the low 32-bit address space, which
causes SWIOTLB on arm64 to fall back to a bounce buffer for DMA
transfers involving memory outside the 32-bit address space.

The R-Car DMA controller hardware supports a 40-bit address space, hence
widen the DMA mask to 40 bits to actually make use of this feature.

Signed-off-by: Geert Uytterhoeven 
---
Note that for proper operation in the presence of an enabled IOMMU, you
also need commit 3b6bb5b705a4051c ("iommu/ipmmu-vmsa: Restrict IOMMU
Domain Geometry to 32-bit address space") in iommu/next.

However, as the IOMMU is not enabled in upstream nor linux-next, it is
safe to apply this patch in parallel.

v2:
  - No changes.
---
 drivers/dma/sh/rcar-dmac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 2e441d0ccd79a37a..93a69b992a51a7aa 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -1716,6 +1716,7 @@ static int rcar_dmac_probe(struct platform_device *pdev)
 
dmac->dev = >dev;
platform_set_drvdata(pdev, dmac);
+   dma_set_mask_and_coherent(dmac->dev, DMA_BIT_MASK(40));
 
ret = rcar_dmac_parse_of(>dev, dmac);
if (ret < 0)
-- 
1.9.1

Re: [PATCH] ARM: shmobile: r7s72100: add restart handler

[Geert Uytterhoeven]

Hi Chris,

On Fri, Feb 10, 2017 at 8:46 PM, Chris Brandt  wrote:
> On Friday, February 10, 2017, Geert Uytterhoeven wrote:
>> Alternatively, you can write a restart driver (cfr.
>> drivers/power/reset/rmobile-reset.c) that binds against a
>> "renesas,r7s72100-wdt" device node, but doesn't implement watchdog
>> functionality.
>> You're gonna need DT bindings anyway.
>
> I like that idea. That should take me no time at all.
> Thank you.
>
> Do you think I can still keep my 'weak function' idea in there??
>
>
> extern void __attribute__ ((weak)) prepare_for_restart(void)
> {
> /* override to do board specific stuff */
> }
>
> static int renesas_wdt_reset_handler(struct notifier_block *this,
>  unsigned long mode, void *cmd)
> {
> pr_debug("%s %lu\n", __func__, mode);
>
> prepare_for_restart();
>
> /* set WDT for reset */
> . . .
>
> return NOTIFY_DONE;
> }

What do you want to use the board-specific function for?
Have a board-specific reset handler, or do some preparatory cleanup?

In case of the former, please write a separate driver that registers  a
reset handler with a higher priority.
In case of the latter, please use register_reboot_notifier() in the driver
that needs it.

> Or...do you think I can just use the rmobile-reset.c driver and
> just add WDT to it?
>
> Honestly, the only thing different will be rmobile_reset_handler().
> I could make a rmobile_wdt_reset_handler() and I could just pass in
> a different notifier_block depending on the DT.
>
> What do you think?

Given the small amount of code to add to the existing driver, and the sheer
amount of boilerplate for writing a new driver, I'm inclined to say yes.
But in the end, it's up to the subsystem maintainer to decide.

> static const struct of_device_id rmobile_reset_of_match[] = {
> { .compatible = "renesas,sysc-rmobile", },
> { .compatible = "renesas,wdt-rmobile", },

renesas,r7s72100-wdt

> { /* sentinel */ }
> };

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/RFC] arm64: defconfig: Enlarge CMA alignment to 2 MiB

[Geert Uytterhoeven]

Hi Robin,

On Wed, Feb 1, 2017 at 3:06 PM, Robin Murphy  wrote:
> On 01/02/17 13:45, Magnus Damm wrote:
>> On Sat, Jan 28, 2017 at 1:03 AM, Geert Uytterhoeven
>>  wrote:
>>> Some IOMMUs (e.g. Renesas IPMMU/VMSA) support only page sizes of 4 KiB,
>>> 2 MiB, and 1 GiB.
>>>
>>> With the default setting of CONFIG_CMA_ALIGNMENT = 8, allocations larger
>>> than 1 MiB are aligned to a 1 MiB boundary only.  Hence a 2 MiB
>>> allocation may not be aligned, leading to a mapping of 512 4 KiB pages.
>>>
>>> Increase CONFIG_CMA_ALIGNMENT to allow mapping a 2 MiB buffer using a
>>> single PTE, decreasing memory usage and TLB pressure.
>>>
>>> Signed-off-by: Geert Uytterhoeven 
>>> ---
>>> Is this useful?
>>>
>>> Should there instead be different defaults in Kconfig, depending on
>>> enabled platform support?
>>
>> I think there is a dependency on the kernel page size configuration as
>> well. In case of 16 KiB or 64 KiB page size configuration other large
>> page sizes may be required.
>
> I don't see the original patch (linux-arm-kernel doesn't look to have it
> archived either), but I think bumping the default up to 2MB to match our

These days linux-arm-kernel (infradead) tends to reject my patches :-(

linux-renesas-soc patchwork does have it:
https://patchwork.kernel.org/patch/9542173/

> normal section size sounds generally reasonable - users can still
> override it manually in their config, right? For 16KB and 64KB granules,
> it is at least the size covered by contiguous-hinted pages; the actual
> section sizes there are far too big to be practical for alignment
> purposes anyway (32MB and 512MB respectively).

Yes, it can be overridden by the user.

One slight annoyance is the dependency on PAGE_SHIFT: if you increase
ARM64_PAGE_SHIFT, you want to decrease CONFIG_CMA_ALIGNMENT.

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: [GIT PULL FOR renesas-drivers] R-Car DU fences support

[Laurent Pinchart]

Hi Geert,

On Monday 13 Feb 2017 11:13:31 Geert Uytterhoeven wrote:
> On Mon, Feb 13, 2017 at 11:05 AM, Geert Uytterhoeven wrote:
> > On Sun, Feb 12, 2017 at 2:13 PM, Laurent Pinchart wrote:
> >> The following changes since commit 
5685cab84a6dc376a8d0e4534ae4b973546158b4:
> >>   Merge remote-tracking branch 'drm-misc/drm-misc-next' into
> >>   drm/next/base
> >> 
> >> (2017-02-12 13:18:50 +0200)
> >> 
> >> are available in the git repository at:
> >>   git://linuxtv.org/pinchartl/media.git drm/next/du
> >> 
> >> for you to fetch changes up to a194138cd82dff52d4c39895fd89dc6f26eafc97:
> >>   drm: rcar-du: Use DRM core's atomic commit helper (2017-02-12 14:15:36
> >> 
> >> +0200)
> > 
> > Thank you, scheduled for inclusion in next renesas-drivers release
> > (probably renesas-drivers-2017-02-21-v4.10).
> 
> Hit send too soon...
> 
> Note that it already fails to merge cleanly into today's linux-next (holding
> wood, rabbit ears and tails, ... for next week).

Hardly a surprise, drm-misc evolves really fast, any patch series that touches 
the DRM core has a high risk of conflicting. I hope that Maarten will resubmit 
his patches soon and get them merged.

> My untested resolution:

Looks good to me.

> diff --cc drivers/gpu/drm/drm_atomic.c
> index a5673107db26c403,428743efc031e717..
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@@ -1938,8 -1949,8 +1949,8 @@@ static int prepare_crtc_signaling(struc
> if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
> return 0;
> 
> -   for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
>  -  u64 __user *fence_ptr;
>  +  s32 __user *fence_ptr;
> 
> fence_ptr = get_out_fence_for_crtc(crtc_state->state, crtc);
> 
> @@@ -2018,17 -2029,14 +2029,17 @@@ static void complete_crtc_signaling(str
> return;
> }
> 
> -   for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
>  +  struct drm_pending_vblank_event *event = crtc_state->event;
> /*
>  -   * TEST_ONLY and PAGE_FLIP_EVENT are mutually
>  -   * exclusive, if they weren't, this code should be
>  -   * called on success for TEST_ONLY too.
>  +   * Free the allocated event. drm_atomic_helper_setup_commit
>  +   * can allocate an event too, so only free it if it's ours
>  +   * to prevent a double free in drm_atomic_state_clear.
>  */
>  -  if (crtc_state->event)
>  -  drm_event_cancel_free(dev,
> _state->event->base); +  if (event && (event->base.fence
> || event->base.file_priv)) { + 
> drm_event_cancel_free(dev, >base);
>  +  crtc_state->event = NULL;
>  +  }
> }
> 
> if (!fence_state)

-- 
Regards,

Laurent Pinchart

Re: [GIT PULL FOR renesas-drivers] R-Car DU fences support

[Geert Uytterhoeven]

Hi Laurent,

On Mon, Feb 13, 2017 at 11:05 AM, Geert Uytterhoeven
 wrote:
> On Sun, Feb 12, 2017 at 2:13 PM, Laurent Pinchart
>  wrote:
>> The following changes since commit 5685cab84a6dc376a8d0e4534ae4b973546158b4:
>>
>>   Merge remote-tracking branch 'drm-misc/drm-misc-next' into drm/next/base
>> (2017-02-12 13:18:50 +0200)
>>
>> are available in the git repository at:
>>
>>   git://linuxtv.org/pinchartl/media.git drm/next/du
>>
>> for you to fetch changes up to a194138cd82dff52d4c39895fd89dc6f26eafc97:
>>
>>   drm: rcar-du: Use DRM core's atomic commit helper (2017-02-12 14:15:36
>> +0200)
>
> Thank you, scheduled for inclusion in next renesas-drivers release
> (probably renesas-drivers-2017-02-21-v4.10).

Hit send too soon...

Note that it already fails to merge cleanly into today's linux-next (holding
wood, rabbit ears and tails, ... for next week). My untested resolution:

diff --cc drivers/gpu/drm/drm_atomic.c
index a5673107db26c403,428743efc031e717..
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@@ -1938,8 -1949,8 +1949,8 @@@ static int prepare_crtc_signaling(struc
if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
return 0;

-   for_each_crtc_in_state(state, crtc, crtc_state, i) {
+   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
 -  u64 __user *fence_ptr;
 +  s32 __user *fence_ptr;

fence_ptr = get_out_fence_for_crtc(crtc_state->state, crtc);

@@@ -2018,17 -2029,14 +2029,17 @@@ static void complete_crtc_signaling(str
return;
}

-   for_each_crtc_in_state(state, crtc, crtc_state, i) {
+   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
 +  struct drm_pending_vblank_event *event = crtc_state->event;
/*
 -   * TEST_ONLY and PAGE_FLIP_EVENT are mutually
 -   * exclusive, if they weren't, this code should be
 -   * called on success for TEST_ONLY too.
 +   * Free the allocated event. drm_atomic_helper_setup_commit
 +   * can allocate an event too, so only free it if it's ours
 +   * to prevent a double free in drm_atomic_state_clear.
 */
 -  if (crtc_state->event)
 -  drm_event_cancel_free(dev, _state->event->base);
 +  if (event && (event->base.fence || event->base.file_priv)) {
 +  drm_event_cancel_free(dev, >base);
 +  crtc_state->event = NULL;
 +  }
}

if (!fence_state)

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: [GIT PULL FOR renesas-drivers] R-Car DU fences support

[Geert Uytterhoeven]

Hi Laurent,

On Sun, Feb 12, 2017 at 2:13 PM, Laurent Pinchart
 wrote:
> The following changes since commit 5685cab84a6dc376a8d0e4534ae4b973546158b4:
>
>   Merge remote-tracking branch 'drm-misc/drm-misc-next' into drm/next/base
> (2017-02-12 13:18:50 +0200)
>
> are available in the git repository at:
>
>   git://linuxtv.org/pinchartl/media.git drm/next/du
>
> for you to fetch changes up to a194138cd82dff52d4c39895fd89dc6f26eafc97:
>
>   drm: rcar-du: Use DRM core's atomic commit helper (2017-02-12 14:15:36
> +0200)

Thank you, scheduled for inclusion in next renesas-drivers release
(probably renesas-drivers-2017-02-21-v4.10).

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] spi: rspi: fix the bug related to mount/remount jffs2

[Geert Uytterhoeven]

On Mon, Feb 6, 2017 at 1:46 PM, Mark Brown  wrote:
> On Mon, Feb 06, 2017 at 10:02:13AM +0900, DongCV wrote:
>> From: Dong 
>>
>> This patch fixes the output warning logs and data loss when
>> performing mount/umount then remount the device with jffs2 format.
>
> This is not a good changelog since it does not describe what the problem
> with the driver is or how the change fixes it - it just says that there
> is a problem.
>
>> This is the warning logs when performing mount/umount then remount the 
>> device with jffs2 format:
>> "root@linaro-naro:~# mount -t jffs2 /dev/mtdblock2 /mnt/media
>> [ 3839.928013] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not 
>> found at 0x03b4: 0x1900 instead
>> [ 3839.956515] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not 
>> found at 0x03b40004: 0x000c instead
>
> Please think hard before including long log messages in upstream
> reports, they often contain almost no useful information relative to
> their size so often obscure the relevant content in your message.  Some
> subset is usually much better (eg, "produces lots of errors like X"
> here).

May I suggest the following:

spi: rspi: Fix bogus received byte in qspi_transfer_in()

When there are less than QSPI_BUFFER_SIZE remaining bytes to be received,
qspi_transfer_in() writes one bogus byte in the receive buffer, possibly
leading to a buffer overflow.

This can be reproduced by mounting, unmounting, and remounting a
jffs2-formatted device, causing lots of warnings like:

jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found
at 0x03b4: 0x1900 instead

Remove the bogus write to fix this.

It's also a good idea to add a Fixes tag:

Fixes: 3be09bec42a800d4 ("spi: rspi: supports 32bytes buffer for
DUAL and QUAD")

(the code was moved afterwards, but both the origin and the move were
 integrated in v4.10-rc1).

Finally:

Reviewed-by: Geert Uytterhoeven 

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] spi: rspi: fix the bug related to mount/remount jffs2

[Geert Uytterhoeven]

Hi Hiep,

On Mon, Feb 13, 2017 at 7:50 AM, Hiep Cao Minh  wrote:
> Sorry to bother you!

No, thank you for finally make me see my wrong reasoning!

>> qspi_transfer_in() does:
>>
>>  while (n > 0) {
>>  len = qspi_set_receive_trigger(rspi, n);
>>  // len will be <= n
>
> I agree. This is len = min value of n and 32.
>>
>>  if (len == QSPI_BUFFER_SIZE) {
>>  // receive blocks of len bytes
>
> Yes, This is len == 32 bytes
>>
>>  ...
>>  } else {
>>  // receive n (not len) bytes
>
> This is always n == len ( This case, len or n is always < 32)
> Because this is the last n bytes should be received.

Right.
Doh, I must have missed that if "len < n", the first branch is always taken.

>>  ret = rspi_pio_transfer(rspi, NULL, rx, n);

It would have been clearer to use "len" here instead of "n".

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