Re: [PATCH v2 2/3] media: imx7-media-csi: add i.MX6UL support

2019-06-11 Thread Rui Miguel Silva 
Hi Sebastien,
On Tue 11 Jun 2019 at 11:03, Sébastien Szymanski wrote:
> On 6/11/19 11:40 AM, Rui Miguel Silva wrote:
>> Hi Sebastien,
>> On Tue 11 Jun 2019 at 09:16, Sébastien Szymanski wrote:
>>> Hi Rui,
>>>
>>> thanks for the review!
>>>
>>> On 6/10/19 12:28 PM, Rui Miguel Silva wrote:
 Hi Sebastien,
 Thanks for the patch.

 On Thu 06 Jun 2019 at 16:38, Sébastien Szymanski wrote:
> i.MX7 and i.MX6UL/L have the same CSI controller. So add i.MX6UL/L support
> to imx7-media-csi driver.
>
> Signed-off-by: Sébastien Szymanski 
> ---
>
> Changes for v2:
>  - rebase on top of linuxtv/master
>  - mention i.MX6UL/L in header and Kconfig help text
>  - rename csi_type to csi_soc_id
>
>  drivers/staging/media/imx/Kconfig  |  4 +-
>  drivers/staging/media/imx/imx7-media-csi.c | 62 --
>  2 files changed, 49 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/staging/media/imx/Kconfig 
> b/drivers/staging/media/imx/Kconfig
> index ad3d7df6bb3c..8b6dc42c39e0 100644
> --- a/drivers/staging/media/imx/Kconfig
> +++ b/drivers/staging/media/imx/Kconfig
> @@ -22,11 +22,11 @@ config VIDEO_IMX_CSI
> A video4linux camera sensor interface driver for i.MX5/6.
>
>  config VIDEO_IMX7_CSI
> - tristate "i.MX7 Camera Sensor Interface driver"
> + tristate "i.MX6UL/L / i.MX7 Camera Sensor Interface driver"
>   depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C
>   default y
>   help
> Enable support for video4linux camera sensor interface driver for
> -   i.MX7.
> +   i.MX6UL/L or i.MX7.
>  endmenu
>  endif
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c 
> b/drivers/staging/media/imx/imx7-media-csi.c
> index 9101566f3f67..902bdce594cf 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - * V4L2 Capture CSI Subdev for Freescale i.MX7 SOC
> + * V4L2 Capture CSI Subdev for Freescale i.MX6UL/L / i.MX7 SOC
>   *
>   * Copyright (c) 2019 Linaro Ltd
>   *
> @@ -152,6 +152,11 @@
>  #define CSI_CSICR18  0x48
>  #define CSI_CSICR19  0x4c
>
> +enum csi_soc_id {
> + IMX7,
> + IMX6UL
> +};
> +
>  struct imx7_csi {
>   struct device *dev;
>   struct v4l2_subdev sd;
> @@ -191,6 +196,7 @@ struct imx7_csi {
>   bool is_init;
>   bool is_streaming;
>   bool is_csi2;
> + enum csi_soc_id soc_id;
>
>   struct completion last_eof_completion;
>  };
> @@ -548,6 +554,14 @@ static int imx7_csi_pad_link_validate(struct 
> v4l2_subdev *sd,
>   if (ret)
>   return ret;
>
> + if (csi->soc_id == IMX6UL) {
> + mutex_lock(>lock);
> + csi->is_csi2 = false;
> + mutex_unlock(>lock);
> +
> + return 0;
> + }
> +
>   ret = imx7_csi_get_upstream_endpoint(csi, _ep, true);
>   if (ret) {
>   v4l2_err(>sd, "failed to find upstream endpoint\n");
> @@ -757,6 +771,7 @@ static int imx7_csi_configure(struct imx7_csi *csi)
>   struct v4l2_pix_format *out_pix = >fmt.fmt.pix;
>   __u32 in_code = csi->format_mbus[IMX7_CSI_PAD_SINK].code;
>   u32 cr1, cr18;
> + int width = out_pix->width;
>
>   if (out_pix->field == V4L2_FIELD_INTERLACED) {
>   imx7_csi_deinterlace_enable(csi, true);
> @@ -766,15 +781,27 @@ static int imx7_csi_configure(struct imx7_csi *csi)
>   imx7_csi_buf_stride_set(csi, 0);
>   }
>
> - imx7_csi_set_imagpara(csi, out_pix->width, out_pix->height);
> + cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
> +
> + if (!csi->is_csi2) {
> + if (out_pix->pixelformat == V4L2_PIX_FMT_UYVY ||
> + out_pix->pixelformat == V4L2_PIX_FMT_YUYV)
> + width *= 2;
> +
> + imx7_csi_set_imagpara(csi, width, out_pix->height);
> +
> + cr18 |= (BIT_BASEADDR_SWITCH_EN | BIT_BASEADDR_SWITCH_SEL |
> + BIT_BASEADDR_CHG_ERR_EN);
> + imx7_csi_reg_write(csi, cr18, CSI_CSICR18);
>
> - if (!csi->is_csi2)
>   return 0;
> + }
> +
> + imx7_csi_set_imagpara(csi, width, out_pix->height);
>
>   cr1 = imx7_csi_reg_read(csi, CSI_CSICR1);
>   cr1 &= ~BIT_GCLK_MODE;
>
> - cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
>   cr18 &= BIT_MIPI_DATA_FORMAT_MASK;
>   cr18 |= BIT_DATA_FROM_MIPI;
>
> @@ -809,11 +836,9 @@ static void imx7_csi_enable(struct imx7_csi *csi)
>  {
>   imx7_csi_sw_reset(csi);
>
> - if (csi->is_csi2) {
> - imx7_csi_dmareq_rff_enable(csi);
> - imx7_csi_hw_enable_irq(csi);
> - imx7_csi_hw_enable(csi);
> - }
> +

Re: [PATCH v2 2/3] media: imx7-media-csi: add i.MX6UL support

2019-06-11 Thread Sébastien Szymanski 
On 6/11/19 11:40 AM, Rui Miguel Silva wrote:
> Hi Sebastien,
> On Tue 11 Jun 2019 at 09:16, Sébastien Szymanski wrote:
>> Hi Rui,
>>
>> thanks for the review!
>>
>> On 6/10/19 12:28 PM, Rui Miguel Silva wrote:
>>> Hi Sebastien,
>>> Thanks for the patch.
>>>
>>> On Thu 06 Jun 2019 at 16:38, Sébastien Szymanski wrote:
 i.MX7 and i.MX6UL/L have the same CSI controller. So add i.MX6UL/L support
 to imx7-media-csi driver.

 Signed-off-by: Sébastien Szymanski 
 ---

 Changes for v2:
  - rebase on top of linuxtv/master
  - mention i.MX6UL/L in header and Kconfig help text
  - rename csi_type to csi_soc_id

  drivers/staging/media/imx/Kconfig  |  4 +-
  drivers/staging/media/imx/imx7-media-csi.c | 62 --
  2 files changed, 49 insertions(+), 17 deletions(-)

 diff --git a/drivers/staging/media/imx/Kconfig 
 b/drivers/staging/media/imx/Kconfig
 index ad3d7df6bb3c..8b6dc42c39e0 100644
 --- a/drivers/staging/media/imx/Kconfig
 +++ b/drivers/staging/media/imx/Kconfig
 @@ -22,11 +22,11 @@ config VIDEO_IMX_CSI
  A video4linux camera sensor interface driver for i.MX5/6.

  config VIDEO_IMX7_CSI
 -  tristate "i.MX7 Camera Sensor Interface driver"
 +  tristate "i.MX6UL/L / i.MX7 Camera Sensor Interface driver"
depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C
default y
help
  Enable support for video4linux camera sensor interface driver for
 -i.MX7.
 +i.MX6UL/L or i.MX7.
  endmenu
  endif
 diff --git a/drivers/staging/media/imx/imx7-media-csi.c 
 b/drivers/staging/media/imx/imx7-media-csi.c
 index 9101566f3f67..902bdce594cf 100644
 --- a/drivers/staging/media/imx/imx7-media-csi.c
 +++ b/drivers/staging/media/imx/imx7-media-csi.c
 @@ -1,6 +1,6 @@
  // SPDX-License-Identifier: GPL-2.0
  /*
 - * V4L2 Capture CSI Subdev for Freescale i.MX7 SOC
 + * V4L2 Capture CSI Subdev for Freescale i.MX6UL/L / i.MX7 SOC
   *
   * Copyright (c) 2019 Linaro Ltd
   *
 @@ -152,6 +152,11 @@
  #define CSI_CSICR18   0x48
  #define CSI_CSICR19   0x4c

 +enum csi_soc_id {
 +  IMX7,
 +  IMX6UL
 +};
 +
  struct imx7_csi {
struct device *dev;
struct v4l2_subdev sd;
 @@ -191,6 +196,7 @@ struct imx7_csi {
bool is_init;
bool is_streaming;
bool is_csi2;
 +  enum csi_soc_id soc_id;

struct completion last_eof_completion;
  };
 @@ -548,6 +554,14 @@ static int imx7_csi_pad_link_validate(struct 
 v4l2_subdev *sd,
if (ret)
return ret;

 +  if (csi->soc_id == IMX6UL) {
 +  mutex_lock(>lock);
 +  csi->is_csi2 = false;
 +  mutex_unlock(>lock);
 +
 +  return 0;
 +  }
 +
ret = imx7_csi_get_upstream_endpoint(csi, _ep, true);
if (ret) {
v4l2_err(>sd, "failed to find upstream endpoint\n");
 @@ -757,6 +771,7 @@ static int imx7_csi_configure(struct imx7_csi *csi)
struct v4l2_pix_format *out_pix = >fmt.fmt.pix;
__u32 in_code = csi->format_mbus[IMX7_CSI_PAD_SINK].code;
u32 cr1, cr18;
 +  int width = out_pix->width;

if (out_pix->field == V4L2_FIELD_INTERLACED) {
imx7_csi_deinterlace_enable(csi, true);
 @@ -766,15 +781,27 @@ static int imx7_csi_configure(struct imx7_csi *csi)
imx7_csi_buf_stride_set(csi, 0);
}

 -  imx7_csi_set_imagpara(csi, out_pix->width, out_pix->height);
 +  cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
 +
 +  if (!csi->is_csi2) {
 +  if (out_pix->pixelformat == V4L2_PIX_FMT_UYVY ||
 +  out_pix->pixelformat == V4L2_PIX_FMT_YUYV)
 +  width *= 2;
 +
 +  imx7_csi_set_imagpara(csi, width, out_pix->height);
 +
 +  cr18 |= (BIT_BASEADDR_SWITCH_EN | BIT_BASEADDR_SWITCH_SEL |
 +  BIT_BASEADDR_CHG_ERR_EN);
 +  imx7_csi_reg_write(csi, cr18, CSI_CSICR18);

 -  if (!csi->is_csi2)
return 0;
 +  }
 +
 +  imx7_csi_set_imagpara(csi, width, out_pix->height);

cr1 = imx7_csi_reg_read(csi, CSI_CSICR1);
cr1 &= ~BIT_GCLK_MODE;

 -  cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
cr18 &= BIT_MIPI_DATA_FORMAT_MASK;
cr18 |= BIT_DATA_FROM_MIPI;

 @@ -809,11 +836,9 @@ static void imx7_csi_enable(struct imx7_csi *csi)
  {
imx7_csi_sw_reset(csi);

 -  if (csi->is_csi2) {
 -  imx7_csi_dmareq_rff_enable(csi);
 -  imx7_csi_hw_enable_irq(csi);
 -  imx7_csi_hw_enable(csi);
 -  }
 +  imx7_csi_dmareq_rff_enable(csi);
 +  imx7_csi_hw_enable_irq(csi);
 +  imx7_csi_hw_enable(csi);
  }

  static void

Re: [PATCH v2 2/3] media: imx7-media-csi: add i.MX6UL support

2019-06-11 Thread Rui Miguel Silva 
Hi Sebastien,
On Tue 11 Jun 2019 at 09:16, Sébastien Szymanski wrote:
> Hi Rui,
>
> thanks for the review!
>
> On 6/10/19 12:28 PM, Rui Miguel Silva wrote:
>> Hi Sebastien,
>> Thanks for the patch.
>>
>> On Thu 06 Jun 2019 at 16:38, Sébastien Szymanski wrote:
>>> i.MX7 and i.MX6UL/L have the same CSI controller. So add i.MX6UL/L support
>>> to imx7-media-csi driver.
>>>
>>> Signed-off-by: Sébastien Szymanski 
>>> ---
>>>
>>> Changes for v2:
>>>  - rebase on top of linuxtv/master
>>>  - mention i.MX6UL/L in header and Kconfig help text
>>>  - rename csi_type to csi_soc_id
>>>
>>>  drivers/staging/media/imx/Kconfig  |  4 +-
>>>  drivers/staging/media/imx/imx7-media-csi.c | 62 --
>>>  2 files changed, 49 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/staging/media/imx/Kconfig 
>>> b/drivers/staging/media/imx/Kconfig
>>> index ad3d7df6bb3c..8b6dc42c39e0 100644
>>> --- a/drivers/staging/media/imx/Kconfig
>>> +++ b/drivers/staging/media/imx/Kconfig
>>> @@ -22,11 +22,11 @@ config VIDEO_IMX_CSI
>>>   A video4linux camera sensor interface driver for i.MX5/6.
>>>
>>>  config VIDEO_IMX7_CSI
>>> -   tristate "i.MX7 Camera Sensor Interface driver"
>>> +   tristate "i.MX6UL/L / i.MX7 Camera Sensor Interface driver"
>>> depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C
>>> default y
>>> help
>>>   Enable support for video4linux camera sensor interface driver for
>>> - i.MX7.
>>> + i.MX6UL/L or i.MX7.
>>>  endmenu
>>>  endif
>>> diff --git a/drivers/staging/media/imx/imx7-media-csi.c 
>>> b/drivers/staging/media/imx/imx7-media-csi.c
>>> index 9101566f3f67..902bdce594cf 100644
>>> --- a/drivers/staging/media/imx/imx7-media-csi.c
>>> +++ b/drivers/staging/media/imx/imx7-media-csi.c
>>> @@ -1,6 +1,6 @@
>>>  // SPDX-License-Identifier: GPL-2.0
>>>  /*
>>> - * V4L2 Capture CSI Subdev for Freescale i.MX7 SOC
>>> + * V4L2 Capture CSI Subdev for Freescale i.MX6UL/L / i.MX7 SOC
>>>   *
>>>   * Copyright (c) 2019 Linaro Ltd
>>>   *
>>> @@ -152,6 +152,11 @@
>>>  #define CSI_CSICR180x48
>>>  #define CSI_CSICR190x4c
>>>
>>> +enum csi_soc_id {
>>> +   IMX7,
>>> +   IMX6UL
>>> +};
>>> +
>>>  struct imx7_csi {
>>> struct device *dev;
>>> struct v4l2_subdev sd;
>>> @@ -191,6 +196,7 @@ struct imx7_csi {
>>> bool is_init;
>>> bool is_streaming;
>>> bool is_csi2;
>>> +   enum csi_soc_id soc_id;
>>>
>>> struct completion last_eof_completion;
>>>  };
>>> @@ -548,6 +554,14 @@ static int imx7_csi_pad_link_validate(struct 
>>> v4l2_subdev *sd,
>>> if (ret)
>>> return ret;
>>>
>>> +   if (csi->soc_id == IMX6UL) {
>>> +   mutex_lock(>lock);
>>> +   csi->is_csi2 = false;
>>> +   mutex_unlock(>lock);
>>> +
>>> +   return 0;
>>> +   }
>>> +
>>> ret = imx7_csi_get_upstream_endpoint(csi, _ep, true);
>>> if (ret) {
>>> v4l2_err(>sd, "failed to find upstream endpoint\n");
>>> @@ -757,6 +771,7 @@ static int imx7_csi_configure(struct imx7_csi *csi)
>>> struct v4l2_pix_format *out_pix = >fmt.fmt.pix;
>>> __u32 in_code = csi->format_mbus[IMX7_CSI_PAD_SINK].code;
>>> u32 cr1, cr18;
>>> +   int width = out_pix->width;
>>>
>>> if (out_pix->field == V4L2_FIELD_INTERLACED) {
>>> imx7_csi_deinterlace_enable(csi, true);
>>> @@ -766,15 +781,27 @@ static int imx7_csi_configure(struct imx7_csi *csi)
>>> imx7_csi_buf_stride_set(csi, 0);
>>> }
>>>
>>> -   imx7_csi_set_imagpara(csi, out_pix->width, out_pix->height);
>>> +   cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
>>> +
>>> +   if (!csi->is_csi2) {
>>> +   if (out_pix->pixelformat == V4L2_PIX_FMT_UYVY ||
>>> +   out_pix->pixelformat == V4L2_PIX_FMT_YUYV)
>>> +   width *= 2;
>>> +
>>> +   imx7_csi_set_imagpara(csi, width, out_pix->height);
>>> +
>>> +   cr18 |= (BIT_BASEADDR_SWITCH_EN | BIT_BASEADDR_SWITCH_SEL |
>>> +   BIT_BASEADDR_CHG_ERR_EN);
>>> +   imx7_csi_reg_write(csi, cr18, CSI_CSICR18);
>>>
>>> -   if (!csi->is_csi2)
>>> return 0;
>>> +   }
>>> +
>>> +   imx7_csi_set_imagpara(csi, width, out_pix->height);
>>>
>>> cr1 = imx7_csi_reg_read(csi, CSI_CSICR1);
>>> cr1 &= ~BIT_GCLK_MODE;
>>>
>>> -   cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
>>> cr18 &= BIT_MIPI_DATA_FORMAT_MASK;
>>> cr18 |= BIT_DATA_FROM_MIPI;
>>>
>>> @@ -809,11 +836,9 @@ static void imx7_csi_enable(struct imx7_csi *csi)
>>>  {
>>> imx7_csi_sw_reset(csi);
>>>
>>> -   if (csi->is_csi2) {
>>> -   imx7_csi_dmareq_rff_enable(csi);
>>> -   imx7_csi_hw_enable_irq(csi);
>>> -   imx7_csi_hw_enable(csi);
>>> -   }
>>> +   imx7_csi_dmareq_rff_enable(csi);
>>> +   imx7_csi_hw_enable_irq(csi);
>>> +   imx7_csi_hw_enable(csi);
>>>  }
>>>
>>>  static void imx7_csi_disable(struct imx7_csi *csi)
>>> @@ -1166,19 +1191,32 @@ static int imx7_csi_parse_endpoint(struct device 
>>> *dev,
>>> return

Re: [PATCH v2 2/3] media: imx7-media-csi: add i.MX6UL support

2019-06-11 Thread Sébastien Szymanski 
Hi Rui,

thanks for the review!

On 6/10/19 12:28 PM, Rui Miguel Silva wrote:
> Hi Sebastien,
> Thanks for the patch.
> 
> On Thu 06 Jun 2019 at 16:38, Sébastien Szymanski wrote:
>> i.MX7 and i.MX6UL/L have the same CSI controller. So add i.MX6UL/L support
>> to imx7-media-csi driver.
>>
>> Signed-off-by: Sébastien Szymanski 
>> ---
>>
>> Changes for v2:
>>  - rebase on top of linuxtv/master
>>  - mention i.MX6UL/L in header and Kconfig help text
>>  - rename csi_type to csi_soc_id
>>
>>  drivers/staging/media/imx/Kconfig  |  4 +-
>>  drivers/staging/media/imx/imx7-media-csi.c | 62 --
>>  2 files changed, 49 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/staging/media/imx/Kconfig 
>> b/drivers/staging/media/imx/Kconfig
>> index ad3d7df6bb3c..8b6dc42c39e0 100644
>> --- a/drivers/staging/media/imx/Kconfig
>> +++ b/drivers/staging/media/imx/Kconfig
>> @@ -22,11 +22,11 @@ config VIDEO_IMX_CSI
>>A video4linux camera sensor interface driver for i.MX5/6.
>>
>>  config VIDEO_IMX7_CSI
>> -tristate "i.MX7 Camera Sensor Interface driver"
>> +tristate "i.MX6UL/L / i.MX7 Camera Sensor Interface driver"
>>  depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C
>>  default y
>>  help
>>Enable support for video4linux camera sensor interface driver for
>> -  i.MX7.
>> +  i.MX6UL/L or i.MX7.
>>  endmenu
>>  endif
>> diff --git a/drivers/staging/media/imx/imx7-media-csi.c 
>> b/drivers/staging/media/imx/imx7-media-csi.c
>> index 9101566f3f67..902bdce594cf 100644
>> --- a/drivers/staging/media/imx/imx7-media-csi.c
>> +++ b/drivers/staging/media/imx/imx7-media-csi.c
>> @@ -1,6 +1,6 @@
>>  // SPDX-License-Identifier: GPL-2.0
>>  /*
>> - * V4L2 Capture CSI Subdev for Freescale i.MX7 SOC
>> + * V4L2 Capture CSI Subdev for Freescale i.MX6UL/L / i.MX7 SOC
>>   *
>>   * Copyright (c) 2019 Linaro Ltd
>>   *
>> @@ -152,6 +152,11 @@
>>  #define CSI_CSICR18 0x48
>>  #define CSI_CSICR19 0x4c
>>
>> +enum csi_soc_id {
>> +IMX7,
>> +IMX6UL
>> +};
>> +
>>  struct imx7_csi {
>>  struct device *dev;
>>  struct v4l2_subdev sd;
>> @@ -191,6 +196,7 @@ struct imx7_csi {
>>  bool is_init;
>>  bool is_streaming;
>>  bool is_csi2;
>> +enum csi_soc_id soc_id;
>>
>>  struct completion last_eof_completion;
>>  };
>> @@ -548,6 +554,14 @@ static int imx7_csi_pad_link_validate(struct 
>> v4l2_subdev *sd,
>>  if (ret)
>>  return ret;
>>
>> +if (csi->soc_id == IMX6UL) {
>> +mutex_lock(>lock);
>> +csi->is_csi2 = false;
>> +mutex_unlock(>lock);
>> +
>> +return 0;
>> +}
>> +
>>  ret = imx7_csi_get_upstream_endpoint(csi, _ep, true);
>>  if (ret) {
>>  v4l2_err(>sd, "failed to find upstream endpoint\n");
>> @@ -757,6 +771,7 @@ static int imx7_csi_configure(struct imx7_csi *csi)
>>  struct v4l2_pix_format *out_pix = >fmt.fmt.pix;
>>  __u32 in_code = csi->format_mbus[IMX7_CSI_PAD_SINK].code;
>>  u32 cr1, cr18;
>> +int width = out_pix->width;
>>
>>  if (out_pix->field == V4L2_FIELD_INTERLACED) {
>>  imx7_csi_deinterlace_enable(csi, true);
>> @@ -766,15 +781,27 @@ static int imx7_csi_configure(struct imx7_csi *csi)
>>  imx7_csi_buf_stride_set(csi, 0);
>>  }
>>
>> -imx7_csi_set_imagpara(csi, out_pix->width, out_pix->height);
>> +cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
>> +
>> +if (!csi->is_csi2) {
>> +if (out_pix->pixelformat == V4L2_PIX_FMT_UYVY ||
>> +out_pix->pixelformat == V4L2_PIX_FMT_YUYV)
>> +width *= 2;
>> +
>> +imx7_csi_set_imagpara(csi, width, out_pix->height);
>> +
>> +cr18 |= (BIT_BASEADDR_SWITCH_EN | BIT_BASEADDR_SWITCH_SEL |
>> +BIT_BASEADDR_CHG_ERR_EN);
>> +imx7_csi_reg_write(csi, cr18, CSI_CSICR18);
>>
>> -if (!csi->is_csi2)
>>  return 0;
>> +}
>> +
>> +imx7_csi_set_imagpara(csi, width, out_pix->height);
>>
>>  cr1 = imx7_csi_reg_read(csi, CSI_CSICR1);
>>  cr1 &= ~BIT_GCLK_MODE;
>>
>> -cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
>>  cr18 &= BIT_MIPI_DATA_FORMAT_MASK;
>>  cr18 |= BIT_DATA_FROM_MIPI;
>>
>> @@ -809,11 +836,9 @@ static void imx7_csi_enable(struct imx7_csi *csi)
>>  {
>>  imx7_csi_sw_reset(csi);
>>
>> -if (csi->is_csi2) {
>> -imx7_csi_dmareq_rff_enable(csi);
>> -imx7_csi_hw_enable_irq(csi);
>> -imx7_csi_hw_enable(csi);
>> -}
>> +imx7_csi_dmareq_rff_enable(csi);
>> +imx7_csi_hw_enable_irq(csi);
>> +imx7_csi_hw_enable(csi);
>>  }
>>
>>  static void imx7_csi_disable(struct imx7_csi *csi)
>> @@ -1166,19 +1191,32 @@ static int imx7_csi_parse_endpoint(struct device 
>> *dev,
>>  return fwnode_device_is_available(asd->match.fwnode) ? 0 : -EINVAL;
>>  }
>>
>> +static const struct of_device_id imx7_csi_of_match[] = {
>> +{ .compatible = "fsl,imx7-csi", .data

Re: [PATCH v2 2/3] media: imx7-media-csi: add i.MX6UL support

2019-06-10 Thread Rui Miguel Silva 
Hi Randy,
On Fri 07 Jun 2019 at 00:10, Randy Dunlap wrote:
> On 6/6/19 8:38 AM, Sébastien Szymanski wrote:
>> i.MX7 and i.MX6UL/L have the same CSI controller. So add i.MX6UL/L support
>> to imx7-media-csi driver.
>>
>> Signed-off-by: Sébastien Szymanski 
>> ---
>>
>> Changes for v2:
>>  - rebase on top of linuxtv/master
>>  - mention i.MX6UL/L in header and Kconfig help text
>>  - rename csi_type to csi_soc_id
>>
>>  drivers/staging/media/imx/Kconfig  |  4 +-
>>  drivers/staging/media/imx/imx7-media-csi.c | 62 --
>>  2 files changed, 49 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/staging/media/imx/Kconfig 
>> b/drivers/staging/media/imx/Kconfig
>> index ad3d7df6bb3c..8b6dc42c39e0 100644
>> --- a/drivers/staging/media/imx/Kconfig
>> +++ b/drivers/staging/media/imx/Kconfig
>> @@ -22,11 +22,11 @@ config VIDEO_IMX_CSI
>>A video4linux camera sensor interface driver for i.MX5/6.
>>
>>  config VIDEO_IMX7_CSI
>> -tristate "i.MX7 Camera Sensor Interface driver"
>> +tristate "i.MX6UL/L / i.MX7 Camera Sensor Interface driver"
>>  depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C
>>  default y
>
> Hi,
> I realize that this "default y" is not part of this patch set, but we have
> pretty strong guidance that a driver should not default to 'y' unless it is
> needed for a system to boot.  If this driver is optional, then please drop
> the 2 occurrences of "default y" in this Kconfig file.

Yeah, even though both depends on imx_media, I agree that they
should not default to y. I will send a patch for this.
Thanks.

---
Cheers,
Rui


>
> thanks.
>>  help
>>Enable support for video4linux camera sensor interface driver for
>> -  i.MX7.
>> +  i.MX6UL/L or i.MX7.
>>  endmenu
>>  endif

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 2/3] media: imx7-media-csi: add i.MX6UL support

2019-06-10 Thread Rui Miguel Silva 
Hi Sebastien,
Thanks for the patch.

On Thu 06 Jun 2019 at 16:38, Sébastien Szymanski wrote:
> i.MX7 and i.MX6UL/L have the same CSI controller. So add i.MX6UL/L support
> to imx7-media-csi driver.
>
> Signed-off-by: Sébastien Szymanski 
> ---
>
> Changes for v2:
>  - rebase on top of linuxtv/master
>  - mention i.MX6UL/L in header and Kconfig help text
>  - rename csi_type to csi_soc_id
>
>  drivers/staging/media/imx/Kconfig  |  4 +-
>  drivers/staging/media/imx/imx7-media-csi.c | 62 --
>  2 files changed, 49 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/staging/media/imx/Kconfig 
> b/drivers/staging/media/imx/Kconfig
> index ad3d7df6bb3c..8b6dc42c39e0 100644
> --- a/drivers/staging/media/imx/Kconfig
> +++ b/drivers/staging/media/imx/Kconfig
> @@ -22,11 +22,11 @@ config VIDEO_IMX_CSI
> A video4linux camera sensor interface driver for i.MX5/6.
>
>  config VIDEO_IMX7_CSI
> - tristate "i.MX7 Camera Sensor Interface driver"
> + tristate "i.MX6UL/L / i.MX7 Camera Sensor Interface driver"
>   depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C
>   default y
>   help
> Enable support for video4linux camera sensor interface driver for
> -   i.MX7.
> +   i.MX6UL/L or i.MX7.
>  endmenu
>  endif
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c 
> b/drivers/staging/media/imx/imx7-media-csi.c
> index 9101566f3f67..902bdce594cf 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - * V4L2 Capture CSI Subdev for Freescale i.MX7 SOC
> + * V4L2 Capture CSI Subdev for Freescale i.MX6UL/L / i.MX7 SOC
>   *
>   * Copyright (c) 2019 Linaro Ltd
>   *
> @@ -152,6 +152,11 @@
>  #define CSI_CSICR18  0x48
>  #define CSI_CSICR19  0x4c
>
> +enum csi_soc_id {
> + IMX7,
> + IMX6UL
> +};
> +
>  struct imx7_csi {
>   struct device *dev;
>   struct v4l2_subdev sd;
> @@ -191,6 +196,7 @@ struct imx7_csi {
>   bool is_init;
>   bool is_streaming;
>   bool is_csi2;
> + enum csi_soc_id soc_id;
>
>   struct completion last_eof_completion;
>  };
> @@ -548,6 +554,14 @@ static int imx7_csi_pad_link_validate(struct v4l2_subdev 
> *sd,
>   if (ret)
>   return ret;
>
> + if (csi->soc_id == IMX6UL) {
> + mutex_lock(>lock);
> + csi->is_csi2 = false;
> + mutex_unlock(>lock);
> +
> + return 0;
> + }
> +
>   ret = imx7_csi_get_upstream_endpoint(csi, _ep, true);
>   if (ret) {
>   v4l2_err(>sd, "failed to find upstream endpoint\n");
> @@ -757,6 +771,7 @@ static int imx7_csi_configure(struct imx7_csi *csi)
>   struct v4l2_pix_format *out_pix = >fmt.fmt.pix;
>   __u32 in_code = csi->format_mbus[IMX7_CSI_PAD_SINK].code;
>   u32 cr1, cr18;
> + int width = out_pix->width;
>
>   if (out_pix->field == V4L2_FIELD_INTERLACED) {
>   imx7_csi_deinterlace_enable(csi, true);
> @@ -766,15 +781,27 @@ static int imx7_csi_configure(struct imx7_csi *csi)
>   imx7_csi_buf_stride_set(csi, 0);
>   }
>
> - imx7_csi_set_imagpara(csi, out_pix->width, out_pix->height);
> + cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
> +
> + if (!csi->is_csi2) {
> + if (out_pix->pixelformat == V4L2_PIX_FMT_UYVY ||
> + out_pix->pixelformat == V4L2_PIX_FMT_YUYV)
> + width *= 2;
> +
> + imx7_csi_set_imagpara(csi, width, out_pix->height);
> +
> + cr18 |= (BIT_BASEADDR_SWITCH_EN | BIT_BASEADDR_SWITCH_SEL |
> + BIT_BASEADDR_CHG_ERR_EN);
> + imx7_csi_reg_write(csi, cr18, CSI_CSICR18);
>
> - if (!csi->is_csi2)
>   return 0;
> + }
> +
> + imx7_csi_set_imagpara(csi, width, out_pix->height);
>
>   cr1 = imx7_csi_reg_read(csi, CSI_CSICR1);
>   cr1 &= ~BIT_GCLK_MODE;
>
> - cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
>   cr18 &= BIT_MIPI_DATA_FORMAT_MASK;
>   cr18 |= BIT_DATA_FROM_MIPI;
>
> @@ -809,11 +836,9 @@ static void imx7_csi_enable(struct imx7_csi *csi)
>  {
>   imx7_csi_sw_reset(csi);
>
> - if (csi->is_csi2) {
> - imx7_csi_dmareq_rff_enable(csi);
> - imx7_csi_hw_enable_irq(csi);
> - imx7_csi_hw_enable(csi);
> - }
> + imx7_csi_dmareq_rff_enable(csi);
> + imx7_csi_hw_enable_irq(csi);
> + imx7_csi_hw_enable(csi);
>  }
>
>  static void imx7_csi_disable(struct imx7_csi *csi)
> @@ -1166,19 +1191,32 @@ static int imx7_csi_parse_endpoint(struct device *dev,
>   return fwnode_device_is_available(asd->match.fwnode) ? 0 : -EINVAL;
>  }
>
> +static const struct of_device_id imx7_csi_of_match[] = {
> + { .compatible = "fsl,imx7-csi", .data = (void *)IMX7 },
> + { .compatible = "fsl,imx6ul-csi", .data = (void *)IMX6UL },

looking at this again I think we can do this is a different way.
Instead

Re: [PATCH v2 2/3] media: imx7-media-csi: add i.MX6UL support

2019-06-06 Thread Randy Dunlap 
On 6/6/19 8:38 AM, Sébastien Szymanski wrote:
> i.MX7 and i.MX6UL/L have the same CSI controller. So add i.MX6UL/L support
> to imx7-media-csi driver.
> 
> Signed-off-by: Sébastien Szymanski 
> ---
> 
> Changes for v2:
>  - rebase on top of linuxtv/master
>  - mention i.MX6UL/L in header and Kconfig help text
>  - rename csi_type to csi_soc_id
> 
>  drivers/staging/media/imx/Kconfig  |  4 +-
>  drivers/staging/media/imx/imx7-media-csi.c | 62 --
>  2 files changed, 49 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/Kconfig 
> b/drivers/staging/media/imx/Kconfig
> index ad3d7df6bb3c..8b6dc42c39e0 100644
> --- a/drivers/staging/media/imx/Kconfig
> +++ b/drivers/staging/media/imx/Kconfig
> @@ -22,11 +22,11 @@ config VIDEO_IMX_CSI
> A video4linux camera sensor interface driver for i.MX5/6.
>  
>  config VIDEO_IMX7_CSI
> - tristate "i.MX7 Camera Sensor Interface driver"
> + tristate "i.MX6UL/L / i.MX7 Camera Sensor Interface driver"
>   depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C
>   default y

Hi,
I realize that this "default y" is not part of this patch set, but we have
pretty strong guidance that a driver should not default to 'y' unless it is
needed for a system to boot.  If this driver is optional, then please drop
the 2 occurrences of "default y" in this Kconfig file.

thanks.
>   help
> Enable support for video4linux camera sensor interface driver for
> -   i.MX7.
> +   i.MX6UL/L or i.MX7.
>  endmenu
>  endif


-- 
~Randy
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 2/3] media: imx7-media-csi: add i.MX6UL support

2019-06-06 Thread Sébastien Szymanski 
i.MX7 and i.MX6UL/L have the same CSI controller. So add i.MX6UL/L support
to imx7-media-csi driver.

Signed-off-by: Sébastien Szymanski 
---

Changes for v2:
 - rebase on top of linuxtv/master
 - mention i.MX6UL/L in header and Kconfig help text
 - rename csi_type to csi_soc_id

 drivers/staging/media/imx/Kconfig  |  4 +-
 drivers/staging/media/imx/imx7-media-csi.c | 62 --
 2 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/media/imx/Kconfig 
b/drivers/staging/media/imx/Kconfig
index ad3d7df6bb3c..8b6dc42c39e0 100644
--- a/drivers/staging/media/imx/Kconfig
+++ b/drivers/staging/media/imx/Kconfig
@@ -22,11 +22,11 @@ config VIDEO_IMX_CSI
  A video4linux camera sensor interface driver for i.MX5/6.
 
 config VIDEO_IMX7_CSI
-   tristate "i.MX7 Camera Sensor Interface driver"
+   tristate "i.MX6UL/L / i.MX7 Camera Sensor Interface driver"
depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C
default y
help
  Enable support for video4linux camera sensor interface driver for
- i.MX7.
+ i.MX6UL/L or i.MX7.
 endmenu
 endif
diff --git a/drivers/staging/media/imx/imx7-media-csi.c 
b/drivers/staging/media/imx/imx7-media-csi.c
index 9101566f3f67..902bdce594cf 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * V4L2 Capture CSI Subdev for Freescale i.MX7 SOC
+ * V4L2 Capture CSI Subdev for Freescale i.MX6UL/L / i.MX7 SOC
  *
  * Copyright (c) 2019 Linaro Ltd
  *
@@ -152,6 +152,11 @@
 #define CSI_CSICR180x48
 #define CSI_CSICR190x4c
 
+enum csi_soc_id {
+   IMX7,
+   IMX6UL
+};
+
 struct imx7_csi {
struct device *dev;
struct v4l2_subdev sd;
@@ -191,6 +196,7 @@ struct imx7_csi {
bool is_init;
bool is_streaming;
bool is_csi2;
+   enum csi_soc_id soc_id;
 
struct completion last_eof_completion;
 };
@@ -548,6 +554,14 @@ static int imx7_csi_pad_link_validate(struct v4l2_subdev 
*sd,
if (ret)
return ret;
 
+   if (csi->soc_id == IMX6UL) {
+   mutex_lock(>lock);
+   csi->is_csi2 = false;
+   mutex_unlock(>lock);
+
+   return 0;
+   }
+
ret = imx7_csi_get_upstream_endpoint(csi, _ep, true);
if (ret) {
v4l2_err(>sd, "failed to find upstream endpoint\n");
@@ -757,6 +771,7 @@ static int imx7_csi_configure(struct imx7_csi *csi)
struct v4l2_pix_format *out_pix = >fmt.fmt.pix;
__u32 in_code = csi->format_mbus[IMX7_CSI_PAD_SINK].code;
u32 cr1, cr18;
+   int width = out_pix->width;
 
if (out_pix->field == V4L2_FIELD_INTERLACED) {
imx7_csi_deinterlace_enable(csi, true);
@@ -766,15 +781,27 @@ static int imx7_csi_configure(struct imx7_csi *csi)
imx7_csi_buf_stride_set(csi, 0);
}
 
-   imx7_csi_set_imagpara(csi, out_pix->width, out_pix->height);
+   cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
+
+   if (!csi->is_csi2) {
+   if (out_pix->pixelformat == V4L2_PIX_FMT_UYVY ||
+   out_pix->pixelformat == V4L2_PIX_FMT_YUYV)
+   width *= 2;
+
+   imx7_csi_set_imagpara(csi, width, out_pix->height);
+
+   cr18 |= (BIT_BASEADDR_SWITCH_EN | BIT_BASEADDR_SWITCH_SEL |
+   BIT_BASEADDR_CHG_ERR_EN);
+   imx7_csi_reg_write(csi, cr18, CSI_CSICR18);
 
-   if (!csi->is_csi2)
return 0;
+   }
+
+   imx7_csi_set_imagpara(csi, width, out_pix->height);
 
cr1 = imx7_csi_reg_read(csi, CSI_CSICR1);
cr1 &= ~BIT_GCLK_MODE;
 
-   cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
cr18 &= BIT_MIPI_DATA_FORMAT_MASK;
cr18 |= BIT_DATA_FROM_MIPI;
 
@@ -809,11 +836,9 @@ static void imx7_csi_enable(struct imx7_csi *csi)
 {
imx7_csi_sw_reset(csi);
 
-   if (csi->is_csi2) {
-   imx7_csi_dmareq_rff_enable(csi);
-   imx7_csi_hw_enable_irq(csi);
-   imx7_csi_hw_enable(csi);
-   }
+   imx7_csi_dmareq_rff_enable(csi);
+   imx7_csi_hw_enable_irq(csi);
+   imx7_csi_hw_enable(csi);
 }
 
 static void imx7_csi_disable(struct imx7_csi *csi)
@@ -1166,19 +1191,32 @@ static int imx7_csi_parse_endpoint(struct device *dev,
return fwnode_device_is_available(asd->match.fwnode) ? 0 : -EINVAL;
 }
 
+static const struct of_device_id imx7_csi_of_match[] = {
+   { .compatible = "fsl,imx7-csi", .data = (void *)IMX7 },
+   { .compatible = "fsl,imx6ul-csi", .data = (void *)IMX6UL },
+   { },
+};
+MODULE_DEVICE_TABLE(of, imx7_csi_of_match);
+
 static int imx7_csi_probe(struct platform_device *pdev)
 {
struct device *dev = >dev;
struct device_node *node = dev->of_node;
struct imx_media_dev *imxmd;
struct imx7_csi *csi;
+   const