Re: [PATCH v2 2/2] mtd: nand: atmel_nand: fix a possible NULL dereference
Hi, Corentin On 11/24/2015 9:12 PM, LABBE Corentin wrote: On Fri, Nov 20, 2015 at 04:33:14PM +0800, Josh Wu wrote: Hi, Corentin Thanks for the patch. It looks fine to me, just one nitpick in below: On 11/20/2015 3:45 PM, LABBE Corentin wrote: of_match_device could return NULL, and so cause a NULL pointer dereference later. Signed-off-by: LABBE Corentin --- drivers/mtd/nand/atmel_nand.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c index 475c938..7902967 100644 --- a/drivers/mtd/nand/atmel_nand.c +++ b/drivers/mtd/nand/atmel_nand.c @@ -1496,8 +1496,9 @@ static int atmel_of_init_port(struct atmel_nand_host *host, struct atmel_nand_data *board = >board; enum of_gpio_flags flags = 0; - host->caps = (struct atmel_nand_caps *) - of_match_device(atmel_nand_dt_ids, host->dev)->data; + host->caps = of_device_get_match_data(host->dev); + if (!host->caps) + return 1; it's better to use -EINVAL here. Hello I do that Uwe Kleine-König said to me to do in others thread: https://lkml.org/lkml/2015/11/12/70 and https://lkml.org/lkml/2015/11/16/211 Thank you for the information. I checked the imx serial driver(drivers/tty/serial/imx.c) in serial_imx_probe(): ... ret = serial_imx_probe_dt(sport, pdev); if (ret > 0) ---> So here is why you need to return 1 in serial_imx_probe_dt(). serial_imx_probe_pdata(sport, pdev); else if (ret < 0) return ret; There has a branch to check whether ret is > 0 or < 0. So that's why you need to return 1 in imx serial driver. But in atmel_nand driver, we don't have such code for that. The atmel_of_init_port() is only used for dt probe. So if dt is not matched, just return an error. Regards Best Regards, Josh Wu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] mtd: nand: atmel_nand: fix a possible NULL dereference
Hi, Corentin On 11/24/2015 9:12 PM, LABBE Corentin wrote: On Fri, Nov 20, 2015 at 04:33:14PM +0800, Josh Wu wrote: Hi, Corentin Thanks for the patch. It looks fine to me, just one nitpick in below: On 11/20/2015 3:45 PM, LABBE Corentin wrote: of_match_device could return NULL, and so cause a NULL pointer dereference later. Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com> --- drivers/mtd/nand/atmel_nand.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c index 475c938..7902967 100644 --- a/drivers/mtd/nand/atmel_nand.c +++ b/drivers/mtd/nand/atmel_nand.c @@ -1496,8 +1496,9 @@ static int atmel_of_init_port(struct atmel_nand_host *host, struct atmel_nand_data *board = >board; enum of_gpio_flags flags = 0; - host->caps = (struct atmel_nand_caps *) - of_match_device(atmel_nand_dt_ids, host->dev)->data; + host->caps = of_device_get_match_data(host->dev); + if (!host->caps) + return 1; it's better to use -EINVAL here. Hello I do that Uwe Kleine-König said to me to do in others thread: https://lkml.org/lkml/2015/11/12/70 and https://lkml.org/lkml/2015/11/16/211 Thank you for the information. I checked the imx serial driver(drivers/tty/serial/imx.c) in serial_imx_probe(): ... ret = serial_imx_probe_dt(sport, pdev); if (ret > 0) ---> So here is why you need to return 1 in serial_imx_probe_dt(). serial_imx_probe_pdata(sport, pdev); else if (ret < 0) return ret; There has a branch to check whether ret is > 0 or < 0. So that's why you need to return 1 in imx serial driver. But in atmel_nand driver, we don't have such code for that. The atmel_of_init_port() is only used for dt probe. So if dt is not matched, just return an error. Regards Best Regards, Josh Wu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] mtd: nand: atmel_nand: fix a possible NULL dereference
Hi, Corentin
Thanks for the patch. It looks fine to me, just one nitpick in below:
On 11/20/2015 3:45 PM, LABBE Corentin wrote:
of_match_device could return NULL, and so cause a NULL pointer
dereference later.
Signed-off-by: LABBE Corentin
---
drivers/mtd/nand/atmel_nand.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 475c938..7902967 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -1496,8 +1496,9 @@ static int atmel_of_init_port(struct atmel_nand_host
*host,
struct atmel_nand_data *board = >board;
enum of_gpio_flags flags = 0;
- host->caps = (struct atmel_nand_caps *)
- of_match_device(atmel_nand_dt_ids, host->dev)->data;
+ host->caps = of_device_get_match_data(host->dev);
+ if (!host->caps)
+ return 1;
it's better to use -EINVAL here.
Best Regards,
Josh Wu
if (of_property_read_u32(np, "atmel,nand-addr-offset", ) == 0) {
if (val >= 32) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/2] mtd: nand: atmel_nand: constify atmel_nand_caps structures
Hi, Corentin
Thanks for the patch.
On 11/20/2015 3:45 PM, LABBE Corentin wrote:
All atmel_nand_caps are never modified, consitify them.
Signed-off-by: LABBE Corentin
Acked-by: Josh Wu
Best Regards,
Josh Wu
---
drivers/mtd/nand/atmel_nand.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 583cdd9..475c938 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -128,7 +128,7 @@ struct atmel_nand_host {
struct atmel_nfc *nfc;
- struct atmel_nand_caps *caps;
+ const struct atmel_nand_caps*caps;
boolhas_pmecc;
u8 pmecc_corr_cap;
u16 pmecc_sector_size;
@@ -2304,11 +2304,11 @@ static int atmel_nand_remove(struct platform_device
*pdev)
return 0;
}
-static struct atmel_nand_caps at91rm9200_caps = {
+static const struct atmel_nand_caps at91rm9200_caps = {
.pmecc_correct_erase_page = false,
};
-static struct atmel_nand_caps sama5d4_caps = {
+static const struct atmel_nand_caps sama5d4_caps = {
.pmecc_correct_erase_page = true,
};
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/2] mtd: nand: atmel_nand: constify atmel_nand_caps structures
Hi, Corentin
Thanks for the patch.
On 11/20/2015 3:45 PM, LABBE Corentin wrote:
All atmel_nand_caps are never modified, consitify them.
Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
Acked-by: Josh Wu <josh...@atmel.com>
Best Regards,
Josh Wu
---
drivers/mtd/nand/atmel_nand.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 583cdd9..475c938 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -128,7 +128,7 @@ struct atmel_nand_host {
struct atmel_nfc *nfc;
- struct atmel_nand_caps *caps;
+ const struct atmel_nand_caps*caps;
boolhas_pmecc;
u8 pmecc_corr_cap;
u16 pmecc_sector_size;
@@ -2304,11 +2304,11 @@ static int atmel_nand_remove(struct platform_device
*pdev)
return 0;
}
-static struct atmel_nand_caps at91rm9200_caps = {
+static const struct atmel_nand_caps at91rm9200_caps = {
.pmecc_correct_erase_page = false,
};
-static struct atmel_nand_caps sama5d4_caps = {
+static const struct atmel_nand_caps sama5d4_caps = {
.pmecc_correct_erase_page = true,
};
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] mtd: nand: atmel_nand: fix a possible NULL dereference
Hi, Corentin
Thanks for the patch. It looks fine to me, just one nitpick in below:
On 11/20/2015 3:45 PM, LABBE Corentin wrote:
of_match_device could return NULL, and so cause a NULL pointer
dereference later.
Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
drivers/mtd/nand/atmel_nand.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 475c938..7902967 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -1496,8 +1496,9 @@ static int atmel_of_init_port(struct atmel_nand_host
*host,
struct atmel_nand_data *board = >board;
enum of_gpio_flags flags = 0;
- host->caps = (struct atmel_nand_caps *)
- of_match_device(atmel_nand_dt_ids, host->dev)->data;
+ host->caps = of_device_get_match_data(host->dev);
+ if (!host->caps)
+ return 1;
it's better to use -EINVAL here.
Best Regards,
Josh Wu
if (of_property_read_u32(np, "atmel,nand-addr-offset", ) == 0) {
if (val >= 32) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/5] media: atmel-isi: prepare for the support of preview path
Atmel ISI support a preview path which can output RGB data.
So this patch introduces a bool variable to choose which path is
enabled currently. And also we need setup corresponding path registers.
By default the preview path is disabled. We only use Codec path.
Signed-off-by: Josh Wu
---
Changes in v2: None
drivers/media/platform/soc_camera/atmel-isi.c | 72 ++-
1 file changed, 49 insertions(+), 23 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index ce87a16..24501a4 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -79,6 +79,7 @@ struct atmel_isi {
dma_addr_t fb_descriptors_phys;
struct list_head dma_desc_head;
struct isi_dma_desc dma_desc[MAX_BUFFER_NUM];
+ boolenable_preview_path;
struct completion complete;
/* ISI peripherial clock */
@@ -195,11 +196,19 @@ static irqreturn_t atmel_isi_handle_streaming(struct
atmel_isi *isi)
/* start next dma frame. */
isi->active = list_entry(isi->video_buffer_list.next,
struct frame_buffer, list);
- isi_writel(isi, ISI_DMA_C_DSCR,
- (u32)isi->active->p_dma_desc->fbd_phys);
- isi_writel(isi, ISI_DMA_C_CTRL,
- ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
- isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_C_CH);
+ if (!isi->enable_preview_path) {
+ isi_writel(isi, ISI_DMA_C_DSCR,
+ (u32)isi->active->p_dma_desc->fbd_phys);
+ isi_writel(isi, ISI_DMA_C_CTRL,
+ ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
+ isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_C_CH);
+ } else {
+ isi_writel(isi, ISI_DMA_P_DSCR,
+ (u32)isi->active->p_dma_desc->fbd_phys);
+ isi_writel(isi, ISI_DMA_P_CTRL,
+ ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
+ isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_P_CH);
+ }
}
return IRQ_HANDLED;
}
@@ -226,7 +235,8 @@ static irqreturn_t isi_interrupt(int irq, void *dev_id)
isi_writel(isi, ISI_INTDIS, ISI_CTRL_DIS);
ret = IRQ_HANDLED;
} else {
- if (likely(pending & ISI_SR_CXFR_DONE))
+ if (likely(pending & ISI_SR_CXFR_DONE) ||
+ likely(pending & ISI_SR_PXFR_DONE))
ret = atmel_isi_handle_streaming(isi);
}
@@ -368,21 +378,35 @@ static void start_dma(struct atmel_isi *isi, struct
frame_buffer *buffer)
ISI_SR_CXFR_DONE | ISI_SR_PXFR_DONE);
/* Check if already in a frame */
- if (isi_readl(isi, ISI_STATUS) & ISI_CTRL_CDC) {
- dev_err(isi->soc_host.icd->parent, "Already in frame
handling.\n");
- return;
- }
+ if (!isi->enable_preview_path) {
+ if (isi_readl(isi, ISI_STATUS) & ISI_CTRL_CDC) {
+ dev_err(isi->soc_host.icd->parent, "Already in frame
handling.\n");
+ return;
+ }
- isi_writel(isi, ISI_DMA_C_DSCR, (u32)buffer->p_dma_desc->fbd_phys);
- isi_writel(isi, ISI_DMA_C_CTRL, ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
- isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_C_CH);
+ isi_writel(isi, ISI_DMA_C_DSCR,
+ (u32)buffer->p_dma_desc->fbd_phys);
+ isi_writel(isi, ISI_DMA_C_CTRL,
+ ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
+ isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_C_CH);
+ } else {
+ isi_writel(isi, ISI_DMA_P_DSCR,
+ (u32)buffer->p_dma_desc->fbd_phys);
+ isi_writel(isi, ISI_DMA_P_CTRL,
+ ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
+ isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_P_CH);
+ }
cfg1 &= ~ISI_CFG1_FRATE_DIV_MASK;
/* Enable linked list */
cfg1 |= isi->pdata.frate | ISI_CFG1_DISCR;
- /* Enable codec path and ISI */
- ctrl = ISI_CTRL_CDC | ISI_CTRL_EN;
+ /* Enable ISI */
+ ctrl = ISI_CTRL_EN;
+
+ if (!isi->enable_preview_path)
+ ctrl |= ISI_CTRL_CDC;
+
isi_writel(isi, ISI_CTRL, ctrl);
isi_writel(isi, ISI_CFG1, cfg1);
}
@@ -458,15 +482,17 @@ static void stop_streaming(struct vb2_queue *vq)
}
[PATCH v2 0/5] media: atmel-isi: enable preview path to output RGB565 format
This series will enable preview path support in atmel-isi. Which can make atmel-isi convert the YUV format (from sensor) to RGB565 format. Changes in v2: - remove the duplicated variable: cfg2_yuv_swap. - correct the comment style - According to Guennadi's suggestion, remove the is_output_rgb() function which only used once. Also move the code into the for loop. Josh Wu (5): media: atmel-isi: correct yuv swap according to different sensor outputs media: atmel-isi: prepare for the support of preview path media: atmel-isi: add code to setup correct resolution for preview path media: atmel-isi: setup YCC_SWAP correctly when using preview path media: atmel-isi: support RGB565 output when sensor output YUV formats drivers/media/platform/soc_camera/atmel-isi.c | 162 +++--- drivers/media/platform/soc_camera/atmel-isi.h | 10 ++ 2 files changed, 132 insertions(+), 40 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 3/5] media: atmel-isi: add code to setup correct resolution for preview path
Not like codec path, preview path can do downsampling, so we should setup
a extra preview width, height for it.
This patch add preview resolution setup without down sampling. So currently
preview path will output same size as sensor output size.
Signed-off-by: Josh Wu
---
Changes in v2: None
drivers/media/platform/soc_camera/atmel-isi.c | 12 +++-
drivers/media/platform/soc_camera/atmel-isi.h | 10 ++
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 24501a4..ae82068 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -131,7 +131,7 @@ static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
static void configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, const struct soc_camera_format_xlate *xlate)
{
- u32 cfg2;
+ u32 cfg2, psize;
/* According to sensor's output format to set cfg2 */
switch (xlate->code) {
@@ -159,6 +159,16 @@ static void configure_geometry(struct atmel_isi *isi, u32
width,
cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
& ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
+
+ /* No down sampling, preview size equal to sensor output size */
+ psize = ((width - 1) << ISI_PSIZE_PREV_HSIZE_OFFSET) &
+ ISI_PSIZE_PREV_HSIZE_MASK;
+ psize |= ((height - 1) << ISI_PSIZE_PREV_VSIZE_OFFSET) &
+ ISI_PSIZE_PREV_VSIZE_MASK;
+ isi_writel(isi, ISI_PSIZE, psize);
+ isi_writel(isi, ISI_PDECF, ISI_PDECF_NO_SAMPLING);
+
+ return;
}
static bool is_supported(struct soc_camera_device *icd,
diff --git a/drivers/media/platform/soc_camera/atmel-isi.h
b/drivers/media/platform/soc_camera/atmel-isi.h
index 5acc771..0acb32a 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.h
+++ b/drivers/media/platform/soc_camera/atmel-isi.h
@@ -79,6 +79,16 @@
#define ISI_CFG2_IM_VSIZE_MASK (0x7FF << ISI_CFG2_IM_VSIZE_OFFSET)
#define ISI_CFG2_IM_HSIZE_MASK (0x7FF << ISI_CFG2_IM_HSIZE_OFFSET)
+/* Bitfields in PSIZE */
+#define ISI_PSIZE_PREV_VSIZE_OFFSET0
+#define ISI_PSIZE_PREV_HSIZE_OFFSET16
+#define ISI_PSIZE_PREV_VSIZE_MASK (0x3FF << ISI_PSIZE_PREV_VSIZE_OFFSET)
+#define ISI_PSIZE_PREV_HSIZE_MASK (0x3FF << ISI_PSIZE_PREV_HSIZE_OFFSET)
+
+/* Bitfields in PDECF */
+#define ISI_PDECF_DEC_FACTOR_MASK (0xFF << 0)
+#defineISI_PDECF_NO_SAMPLING (16)
+
/* Bitfields in CTRL */
/* Also using in SR(ISI_V2) */
#define ISI_CTRL_EN(1 << 0)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 5/5] media: atmel-isi: support RGB565 output when sensor output YUV formats
This patch enable Atmel ISI preview path to convert the YUV to RGB format.
Signed-off-by: Josh Wu
---
Changes in v2:
- According to Guennadi's suggestion, remove the is_output_rgb() function
which only used once. Also move the code into the for loop.
drivers/media/platform/soc_camera/atmel-isi.c | 25 +++--
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 826d04e..8abeeeb 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -146,6 +146,10 @@ static void configure_geometry(struct atmel_isi *isi, u32
width,
u32 height, const struct soc_camera_format_xlate *xlate)
{
u32 cfg2, psize;
+ u32 fourcc = xlate->host_fmt->fourcc;
+
+ isi->enable_preview_path = (fourcc == V4L2_PIX_FMT_RGB565 ||
+ fourcc == V4L2_PIX_FMT_RGB32);
/* According to sensor's output format to set cfg2 */
switch (xlate->code) {
@@ -195,8 +199,9 @@ static bool is_supported(struct soc_camera_device *icd,
case V4L2_PIX_FMT_UYVY:
case V4L2_PIX_FMT_YVYU:
case V4L2_PIX_FMT_VYUY:
+ /* RGB */
+ case V4L2_PIX_FMT_RGB565:
return true;
- /* RGB, TODO */
default:
return false;
}
@@ -682,6 +687,14 @@ static const struct soc_mbus_pixelfmt isi_camera_formats[]
= {
.order = SOC_MBUS_ORDER_LE,
.layout = SOC_MBUS_LAYOUT_PACKED,
},
+ {
+ .fourcc = V4L2_PIX_FMT_RGB565,
+ .name = "RGB565",
+ .bits_per_sample= 8,
+ .packing= SOC_MBUS_PACKING_2X8_PADHI,
+ .order = SOC_MBUS_ORDER_LE,
+ .layout = SOC_MBUS_LAYOUT_PACKED,
+ },
};
/* This will be corrected as we get more formats */
@@ -738,7 +751,7 @@ static int isi_camera_get_formats(struct soc_camera_device
*icd,
struct soc_camera_format_xlate *xlate)
{
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
- int formats = 0, ret;
+ int formats = 0, ret, i, n;
/* sensor format */
struct v4l2_subdev_mbus_code_enum code = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
@@ -772,11 +785,11 @@ static int isi_camera_get_formats(struct
soc_camera_device *icd,
case MEDIA_BUS_FMT_VYUY8_2X8:
case MEDIA_BUS_FMT_YUYV8_2X8:
case MEDIA_BUS_FMT_YVYU8_2X8:
- formats++;
- if (xlate) {
- xlate->host_fmt = _camera_formats[0];
+ n = ARRAY_SIZE(isi_camera_formats);
+ formats += n;
+ for (i = 0; xlate && i < n; i++, xlate++) {
+ xlate->host_fmt = _camera_formats[i];
xlate->code = code.code;
- xlate++;
dev_dbg(icd->parent, "Providing format %s using code
%d\n",
isi_camera_formats[0].name, code.code);
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/5] media: atmel-isi: correct yuv swap according to different sensor outputs
we need to configure the YCC_SWAP bits in ISI_CFG2 according to current
sensor output and Atmel ISI output format.
Current there are two cases Atmel ISI supported:
1. Atmel ISI outputs YUYV format.
This case we need to setup YCC_SWAP according to sensor output
format.
2. Atmel ISI output a pass-through formats, which means no swap.
Just setup YCC_SWAP as default with no swap.
Signed-off-by: Josh Wu
---
Changes in v2:
- remove the duplicated variable: cfg2_yuv_swap.
drivers/media/platform/soc_camera/atmel-isi.c | 39 ---
1 file changed, 29 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 45e304a..ce87a16 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -103,13 +103,37 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
return readl(isi->regs + reg);
}
+static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
+ const struct soc_camera_format_xlate *xlate)
+{
+ if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_YUYV) {
+ /* all convert to YUYV */
+ switch (xlate->code) {
+ case MEDIA_BUS_FMT_VYUY8_2X8:
+ return ISI_CFG2_YCC_SWAP_MODE_3;
+ case MEDIA_BUS_FMT_UYVY8_2X8:
+ return ISI_CFG2_YCC_SWAP_MODE_2;
+ case MEDIA_BUS_FMT_YVYU8_2X8:
+ return ISI_CFG2_YCC_SWAP_MODE_1;
+ }
+ }
+
+ /*
+* By default, no swap for the codec path of Atmel ISI. So codec
+* output is same as sensor's output.
+* For instance, if sensor's output is YUYV, then codec outputs YUYV.
+* And if sensor's output is UYVY, then codec outputs UYVY.
+*/
+ return ISI_CFG2_YCC_SWAP_DEFAULT;
+}
+
static void configure_geometry(struct atmel_isi *isi, u32 width,
- u32 height, u32 code)
+ u32 height, const struct soc_camera_format_xlate *xlate)
{
u32 cfg2;
/* According to sensor's output format to set cfg2 */
- switch (code) {
+ switch (xlate->code) {
default:
/* Grey */
case MEDIA_BUS_FMT_Y8_1X8:
@@ -117,16 +141,11 @@ static void configure_geometry(struct atmel_isi *isi, u32
width,
break;
/* YUV */
case MEDIA_BUS_FMT_VYUY8_2X8:
- cfg2 = ISI_CFG2_YCC_SWAP_MODE_3 | ISI_CFG2_COL_SPACE_YCbCr;
- break;
case MEDIA_BUS_FMT_UYVY8_2X8:
- cfg2 = ISI_CFG2_YCC_SWAP_MODE_2 | ISI_CFG2_COL_SPACE_YCbCr;
- break;
case MEDIA_BUS_FMT_YVYU8_2X8:
- cfg2 = ISI_CFG2_YCC_SWAP_MODE_1 | ISI_CFG2_COL_SPACE_YCbCr;
- break;
case MEDIA_BUS_FMT_YUYV8_2X8:
- cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT | ISI_CFG2_COL_SPACE_YCbCr;
+ cfg2 = ISI_CFG2_COL_SPACE_YCbCr |
+ setup_cfg2_yuv_swap(isi, xlate);
break;
/* RGB, TODO */
}
@@ -407,7 +426,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned
int count)
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
configure_geometry(isi, icd->user_width, icd->user_height,
- icd->current_fmt->code);
+ icd->current_fmt);
spin_lock_irq(>lock);
/* Clear any pending interrupt */
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 4/5] media: atmel-isi: setup YCC_SWAP correctly when using preview path
The preview path only can convert UYVY format to RGB data.
To make preview path work correctly, we need to set up YCC_SWAP
according to sensor output and convert them to UYVY.
Signed-off-by: Josh Wu
---
Changes in v2:
- remove cfg2_yuv_swap for rgb format
- correct the comment style
drivers/media/platform/soc_camera/atmel-isi.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index ae82068..826d04e 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -117,6 +117,20 @@ static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
case MEDIA_BUS_FMT_YVYU8_2X8:
return ISI_CFG2_YCC_SWAP_MODE_1;
}
+ } else if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_RGB565) {
+ /*
+* Preview path is enabled, it will convert UYVY to RGB format.
+* But if sensor output format is not UYVY, we need to set
+* YCC_SWAP_MODE to convert it as UYVY.
+*/
+ switch (xlate->code) {
+ case MEDIA_BUS_FMT_VYUY8_2X8:
+ return ISI_CFG2_YCC_SWAP_MODE_1;
+ case MEDIA_BUS_FMT_YUYV8_2X8:
+ return ISI_CFG2_YCC_SWAP_MODE_2;
+ case MEDIA_BUS_FMT_YVYU8_2X8:
+ return ISI_CFG2_YCC_SWAP_MODE_3;
+ }
}
/*
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/5] media: atmel-isi: correct yuv swap according to different sensor outputs
we need to configure the YCC_SWAP bits in ISI_CFG2 according to current
sensor output and Atmel ISI output format.
Current there are two cases Atmel ISI supported:
1. Atmel ISI outputs YUYV format.
This case we need to setup YCC_SWAP according to sensor output
format.
2. Atmel ISI output a pass-through formats, which means no swap.
Just setup YCC_SWAP as default with no swap.
Signed-off-by: Josh Wu <josh...@atmel.com>
---
Changes in v2:
- remove the duplicated variable: cfg2_yuv_swap.
drivers/media/platform/soc_camera/atmel-isi.c | 39 ---
1 file changed, 29 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 45e304a..ce87a16 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -103,13 +103,37 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
return readl(isi->regs + reg);
}
+static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
+ const struct soc_camera_format_xlate *xlate)
+{
+ if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_YUYV) {
+ /* all convert to YUYV */
+ switch (xlate->code) {
+ case MEDIA_BUS_FMT_VYUY8_2X8:
+ return ISI_CFG2_YCC_SWAP_MODE_3;
+ case MEDIA_BUS_FMT_UYVY8_2X8:
+ return ISI_CFG2_YCC_SWAP_MODE_2;
+ case MEDIA_BUS_FMT_YVYU8_2X8:
+ return ISI_CFG2_YCC_SWAP_MODE_1;
+ }
+ }
+
+ /*
+* By default, no swap for the codec path of Atmel ISI. So codec
+* output is same as sensor's output.
+* For instance, if sensor's output is YUYV, then codec outputs YUYV.
+* And if sensor's output is UYVY, then codec outputs UYVY.
+*/
+ return ISI_CFG2_YCC_SWAP_DEFAULT;
+}
+
static void configure_geometry(struct atmel_isi *isi, u32 width,
- u32 height, u32 code)
+ u32 height, const struct soc_camera_format_xlate *xlate)
{
u32 cfg2;
/* According to sensor's output format to set cfg2 */
- switch (code) {
+ switch (xlate->code) {
default:
/* Grey */
case MEDIA_BUS_FMT_Y8_1X8:
@@ -117,16 +141,11 @@ static void configure_geometry(struct atmel_isi *isi, u32
width,
break;
/* YUV */
case MEDIA_BUS_FMT_VYUY8_2X8:
- cfg2 = ISI_CFG2_YCC_SWAP_MODE_3 | ISI_CFG2_COL_SPACE_YCbCr;
- break;
case MEDIA_BUS_FMT_UYVY8_2X8:
- cfg2 = ISI_CFG2_YCC_SWAP_MODE_2 | ISI_CFG2_COL_SPACE_YCbCr;
- break;
case MEDIA_BUS_FMT_YVYU8_2X8:
- cfg2 = ISI_CFG2_YCC_SWAP_MODE_1 | ISI_CFG2_COL_SPACE_YCbCr;
- break;
case MEDIA_BUS_FMT_YUYV8_2X8:
- cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT | ISI_CFG2_COL_SPACE_YCbCr;
+ cfg2 = ISI_CFG2_COL_SPACE_YCbCr |
+ setup_cfg2_yuv_swap(isi, xlate);
break;
/* RGB, TODO */
}
@@ -407,7 +426,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned
int count)
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
configure_geometry(isi, icd->user_width, icd->user_height,
- icd->current_fmt->code);
+ icd->current_fmt);
spin_lock_irq(>lock);
/* Clear any pending interrupt */
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 3/5] media: atmel-isi: add code to setup correct resolution for preview path
Not like codec path, preview path can do downsampling, so we should setup
a extra preview width, height for it.
This patch add preview resolution setup without down sampling. So currently
preview path will output same size as sensor output size.
Signed-off-by: Josh Wu <josh...@atmel.com>
---
Changes in v2: None
drivers/media/platform/soc_camera/atmel-isi.c | 12 +++-
drivers/media/platform/soc_camera/atmel-isi.h | 10 ++
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 24501a4..ae82068 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -131,7 +131,7 @@ static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
static void configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, const struct soc_camera_format_xlate *xlate)
{
- u32 cfg2;
+ u32 cfg2, psize;
/* According to sensor's output format to set cfg2 */
switch (xlate->code) {
@@ -159,6 +159,16 @@ static void configure_geometry(struct atmel_isi *isi, u32
width,
cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
& ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
+
+ /* No down sampling, preview size equal to sensor output size */
+ psize = ((width - 1) << ISI_PSIZE_PREV_HSIZE_OFFSET) &
+ ISI_PSIZE_PREV_HSIZE_MASK;
+ psize |= ((height - 1) << ISI_PSIZE_PREV_VSIZE_OFFSET) &
+ ISI_PSIZE_PREV_VSIZE_MASK;
+ isi_writel(isi, ISI_PSIZE, psize);
+ isi_writel(isi, ISI_PDECF, ISI_PDECF_NO_SAMPLING);
+
+ return;
}
static bool is_supported(struct soc_camera_device *icd,
diff --git a/drivers/media/platform/soc_camera/atmel-isi.h
b/drivers/media/platform/soc_camera/atmel-isi.h
index 5acc771..0acb32a 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.h
+++ b/drivers/media/platform/soc_camera/atmel-isi.h
@@ -79,6 +79,16 @@
#define ISI_CFG2_IM_VSIZE_MASK (0x7FF << ISI_CFG2_IM_VSIZE_OFFSET)
#define ISI_CFG2_IM_HSIZE_MASK (0x7FF << ISI_CFG2_IM_HSIZE_OFFSET)
+/* Bitfields in PSIZE */
+#define ISI_PSIZE_PREV_VSIZE_OFFSET0
+#define ISI_PSIZE_PREV_HSIZE_OFFSET16
+#define ISI_PSIZE_PREV_VSIZE_MASK (0x3FF << ISI_PSIZE_PREV_VSIZE_OFFSET)
+#define ISI_PSIZE_PREV_HSIZE_MASK (0x3FF << ISI_PSIZE_PREV_HSIZE_OFFSET)
+
+/* Bitfields in PDECF */
+#define ISI_PDECF_DEC_FACTOR_MASK (0xFF << 0)
+#defineISI_PDECF_NO_SAMPLING (16)
+
/* Bitfields in CTRL */
/* Also using in SR(ISI_V2) */
#define ISI_CTRL_EN(1 << 0)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 4/5] media: atmel-isi: setup YCC_SWAP correctly when using preview path
The preview path only can convert UYVY format to RGB data.
To make preview path work correctly, we need to set up YCC_SWAP
according to sensor output and convert them to UYVY.
Signed-off-by: Josh Wu <josh...@atmel.com>
---
Changes in v2:
- remove cfg2_yuv_swap for rgb format
- correct the comment style
drivers/media/platform/soc_camera/atmel-isi.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index ae82068..826d04e 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -117,6 +117,20 @@ static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
case MEDIA_BUS_FMT_YVYU8_2X8:
return ISI_CFG2_YCC_SWAP_MODE_1;
}
+ } else if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_RGB565) {
+ /*
+* Preview path is enabled, it will convert UYVY to RGB format.
+* But if sensor output format is not UYVY, we need to set
+* YCC_SWAP_MODE to convert it as UYVY.
+*/
+ switch (xlate->code) {
+ case MEDIA_BUS_FMT_VYUY8_2X8:
+ return ISI_CFG2_YCC_SWAP_MODE_1;
+ case MEDIA_BUS_FMT_YUYV8_2X8:
+ return ISI_CFG2_YCC_SWAP_MODE_2;
+ case MEDIA_BUS_FMT_YVYU8_2X8:
+ return ISI_CFG2_YCC_SWAP_MODE_3;
+ }
}
/*
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 5/5] media: atmel-isi: support RGB565 output when sensor output YUV formats
This patch enable Atmel ISI preview path to convert the YUV to RGB format.
Signed-off-by: Josh Wu <josh...@atmel.com>
---
Changes in v2:
- According to Guennadi's suggestion, remove the is_output_rgb() function
which only used once. Also move the code into the for loop.
drivers/media/platform/soc_camera/atmel-isi.c | 25 +++--
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 826d04e..8abeeeb 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -146,6 +146,10 @@ static void configure_geometry(struct atmel_isi *isi, u32
width,
u32 height, const struct soc_camera_format_xlate *xlate)
{
u32 cfg2, psize;
+ u32 fourcc = xlate->host_fmt->fourcc;
+
+ isi->enable_preview_path = (fourcc == V4L2_PIX_FMT_RGB565 ||
+ fourcc == V4L2_PIX_FMT_RGB32);
/* According to sensor's output format to set cfg2 */
switch (xlate->code) {
@@ -195,8 +199,9 @@ static bool is_supported(struct soc_camera_device *icd,
case V4L2_PIX_FMT_UYVY:
case V4L2_PIX_FMT_YVYU:
case V4L2_PIX_FMT_VYUY:
+ /* RGB */
+ case V4L2_PIX_FMT_RGB565:
return true;
- /* RGB, TODO */
default:
return false;
}
@@ -682,6 +687,14 @@ static const struct soc_mbus_pixelfmt isi_camera_formats[]
= {
.order = SOC_MBUS_ORDER_LE,
.layout = SOC_MBUS_LAYOUT_PACKED,
},
+ {
+ .fourcc = V4L2_PIX_FMT_RGB565,
+ .name = "RGB565",
+ .bits_per_sample= 8,
+ .packing= SOC_MBUS_PACKING_2X8_PADHI,
+ .order = SOC_MBUS_ORDER_LE,
+ .layout = SOC_MBUS_LAYOUT_PACKED,
+ },
};
/* This will be corrected as we get more formats */
@@ -738,7 +751,7 @@ static int isi_camera_get_formats(struct soc_camera_device
*icd,
struct soc_camera_format_xlate *xlate)
{
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
- int formats = 0, ret;
+ int formats = 0, ret, i, n;
/* sensor format */
struct v4l2_subdev_mbus_code_enum code = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
@@ -772,11 +785,11 @@ static int isi_camera_get_formats(struct
soc_camera_device *icd,
case MEDIA_BUS_FMT_VYUY8_2X8:
case MEDIA_BUS_FMT_YUYV8_2X8:
case MEDIA_BUS_FMT_YVYU8_2X8:
- formats++;
- if (xlate) {
- xlate->host_fmt = _camera_formats[0];
+ n = ARRAY_SIZE(isi_camera_formats);
+ formats += n;
+ for (i = 0; xlate && i < n; i++, xlate++) {
+ xlate->host_fmt = _camera_formats[i];
xlate->code = code.code;
- xlate++;
dev_dbg(icd->parent, "Providing format %s using code
%d\n",
isi_camera_formats[0].name, code.code);
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/5] media: atmel-isi: prepare for the support of preview path
Atmel ISI support a preview path which can output RGB data.
So this patch introduces a bool variable to choose which path is
enabled currently. And also we need setup corresponding path registers.
By default the preview path is disabled. We only use Codec path.
Signed-off-by: Josh Wu <josh...@atmel.com>
---
Changes in v2: None
drivers/media/platform/soc_camera/atmel-isi.c | 72 ++-
1 file changed, 49 insertions(+), 23 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index ce87a16..24501a4 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -79,6 +79,7 @@ struct atmel_isi {
dma_addr_t fb_descriptors_phys;
struct list_head dma_desc_head;
struct isi_dma_desc dma_desc[MAX_BUFFER_NUM];
+ boolenable_preview_path;
struct completion complete;
/* ISI peripherial clock */
@@ -195,11 +196,19 @@ static irqreturn_t atmel_isi_handle_streaming(struct
atmel_isi *isi)
/* start next dma frame. */
isi->active = list_entry(isi->video_buffer_list.next,
struct frame_buffer, list);
- isi_writel(isi, ISI_DMA_C_DSCR,
- (u32)isi->active->p_dma_desc->fbd_phys);
- isi_writel(isi, ISI_DMA_C_CTRL,
- ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
- isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_C_CH);
+ if (!isi->enable_preview_path) {
+ isi_writel(isi, ISI_DMA_C_DSCR,
+ (u32)isi->active->p_dma_desc->fbd_phys);
+ isi_writel(isi, ISI_DMA_C_CTRL,
+ ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
+ isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_C_CH);
+ } else {
+ isi_writel(isi, ISI_DMA_P_DSCR,
+ (u32)isi->active->p_dma_desc->fbd_phys);
+ isi_writel(isi, ISI_DMA_P_CTRL,
+ ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
+ isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_P_CH);
+ }
}
return IRQ_HANDLED;
}
@@ -226,7 +235,8 @@ static irqreturn_t isi_interrupt(int irq, void *dev_id)
isi_writel(isi, ISI_INTDIS, ISI_CTRL_DIS);
ret = IRQ_HANDLED;
} else {
- if (likely(pending & ISI_SR_CXFR_DONE))
+ if (likely(pending & ISI_SR_CXFR_DONE) ||
+ likely(pending & ISI_SR_PXFR_DONE))
ret = atmel_isi_handle_streaming(isi);
}
@@ -368,21 +378,35 @@ static void start_dma(struct atmel_isi *isi, struct
frame_buffer *buffer)
ISI_SR_CXFR_DONE | ISI_SR_PXFR_DONE);
/* Check if already in a frame */
- if (isi_readl(isi, ISI_STATUS) & ISI_CTRL_CDC) {
- dev_err(isi->soc_host.icd->parent, "Already in frame
handling.\n");
- return;
- }
+ if (!isi->enable_preview_path) {
+ if (isi_readl(isi, ISI_STATUS) & ISI_CTRL_CDC) {
+ dev_err(isi->soc_host.icd->parent, "Already in frame
handling.\n");
+ return;
+ }
- isi_writel(isi, ISI_DMA_C_DSCR, (u32)buffer->p_dma_desc->fbd_phys);
- isi_writel(isi, ISI_DMA_C_CTRL, ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
- isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_C_CH);
+ isi_writel(isi, ISI_DMA_C_DSCR,
+ (u32)buffer->p_dma_desc->fbd_phys);
+ isi_writel(isi, ISI_DMA_C_CTRL,
+ ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
+ isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_C_CH);
+ } else {
+ isi_writel(isi, ISI_DMA_P_DSCR,
+ (u32)buffer->p_dma_desc->fbd_phys);
+ isi_writel(isi, ISI_DMA_P_CTRL,
+ ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
+ isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_P_CH);
+ }
cfg1 &= ~ISI_CFG1_FRATE_DIV_MASK;
/* Enable linked list */
cfg1 |= isi->pdata.frate | ISI_CFG1_DISCR;
- /* Enable codec path and ISI */
- ctrl = ISI_CTRL_CDC | ISI_CTRL_EN;
+ /* Enable ISI */
+ ctrl = ISI_CTRL_EN;
+
+ if (!isi->enable_preview_path)
+ ctrl |= ISI_CTRL_CDC;
+
isi_writel(isi, ISI_CTRL, ctrl);
isi_writel(isi, ISI_CFG1, cfg1);
}
@@ -458,15 +482,17 @@ static void stop_streaming(struct vb2_queue *vq)
[PATCH v2 0/5] media: atmel-isi: enable preview path to output RGB565 format
This series will enable preview path support in atmel-isi. Which can make atmel-isi convert the YUV format (from sensor) to RGB565 format. Changes in v2: - remove the duplicated variable: cfg2_yuv_swap. - correct the comment style - According to Guennadi's suggestion, remove the is_output_rgb() function which only used once. Also move the code into the for loop. Josh Wu (5): media: atmel-isi: correct yuv swap according to different sensor outputs media: atmel-isi: prepare for the support of preview path media: atmel-isi: add code to setup correct resolution for preview path media: atmel-isi: setup YCC_SWAP correctly when using preview path media: atmel-isi: support RGB565 output when sensor output YUV formats drivers/media/platform/soc_camera/atmel-isi.c | 162 +++--- drivers/media/platform/soc_camera/atmel-isi.h | 10 ++ 2 files changed, 132 insertions(+), 40 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/4] v4l2-clk: add new macro for v4l2_clk_name_of()
This macro is used to generate a OF string for a v4l2 clock.
Signed-off-by: Josh Wu
---
drivers/media/platform/soc_camera/soc_camera.c | 4 ++--
include/media/v4l2-clk.h | 3 +++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index d165bff..673f1d4 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1680,8 +1680,8 @@ static int soc_of_bind(struct soc_camera_host *ici,
v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
client->adapter->nr, client->addr);
else
- snprintf(clk_name, sizeof(clk_name), "of-%s",
-of_node_full_name(remote));
+ v4l2_clk_name_of(clk_name, sizeof(clk_name),
+of_node_full_name(remote));
icd->clk = v4l2_clk_register(_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd->clk)) {
diff --git a/include/media/v4l2-clk.h b/include/media/v4l2-clk.h
index 3ef6e3d..34891ea 100644
--- a/include/media/v4l2-clk.h
+++ b/include/media/v4l2-clk.h
@@ -68,4 +68,7 @@ static inline struct v4l2_clk *v4l2_clk_register_fixed(const
char *dev_id,
#define v4l2_clk_name_i2c(name, size, adap, client) snprintf(name, size, \
"%d-%04x", adap, client)
+#define v4l2_clk_name_of(name, size, of_full_name) snprintf(name, size, \
+ "of-%s", of_full_name)
+
#endif
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/4] v4l2-clk: v4l2_clk_get() also need to find the of_fullname clock
The soc-camera host will be probed and register a v4l2_clk, but if on
that moment, the i2c device is not available, then the registered
v4l2_clk name is a OF string not a I2C string.
So when i2c sensor probed and call v4l2_clk_get(), it only search the
clock with I2C string, like "1-0030".
This patch will search the clock with OF string name if fail to find the
clock with I2C string name.
Signed-off-by: Josh Wu
---
drivers/media/v4l2-core/v4l2-clk.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-clk.c
b/drivers/media/v4l2-core/v4l2-clk.c
index 34e416a..297e10e 100644
--- a/drivers/media/v4l2-core/v4l2-clk.c
+++ b/drivers/media/v4l2-core/v4l2-clk.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
@@ -39,6 +40,7 @@ struct v4l2_clk *v4l2_clk_get(struct device *dev, const char
*id)
{
struct v4l2_clk *clk;
struct clk *ccf_clk = clk_get(dev, id);
+ char clk_name[V4L2_CLK_NAME_SIZE];
if (PTR_ERR(ccf_clk) == -EPROBE_DEFER)
return ERR_PTR(-EPROBE_DEFER);
@@ -57,6 +59,13 @@ struct v4l2_clk *v4l2_clk_get(struct device *dev, const char
*id)
mutex_lock(_lock);
clk = v4l2_clk_find(dev_name(dev));
+ /* if dev_name is not found, try use the OF name to find again */
+ if (PTR_ERR(clk) == -ENODEV && dev->of_node) {
+ v4l2_clk_name_of(clk_name, sizeof(clk_name),
+of_node_full_name(dev->of_node));
+ clk = v4l2_clk_find(clk_name);
+ }
+
if (!IS_ERR(clk))
atomic_inc(>use_count);
mutex_unlock(_lock);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/4] v4l2-clk: add new definition: V4L2_CLK_NAME_SIZE
Make all v4l2-clk's clock name use V4L2_CLK_NAME_SIZE definition.
In future, if the string increased we just need to change the
V4L2_CLK_NAME_SIZE once.
Signed-off-by: Josh Wu
---
drivers/media/platform/soc_camera/soc_camera.c | 6 +++---
drivers/media/usb/em28xx/em28xx-camera.c | 2 +-
include/media/v4l2-clk.h | 2 ++
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index 673f1d4..506a569 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1362,7 +1362,7 @@ static int soc_camera_i2c_init(struct soc_camera_device
*icd,
struct soc_camera_host_desc *shd = >host_desc;
struct i2c_adapter *adap;
struct v4l2_subdev *subdev;
- char clk_name[V4L2_SUBDEV_NAME_SIZE];
+ char clk_name[V4L2_CLK_NAME_SIZE];
int ret;
/* First find out how we link the main client */
@@ -1528,7 +1528,7 @@ static int scan_async_group(struct soc_camera_host *ici,
struct soc_camera_async_client *sasc;
struct soc_camera_device *icd;
struct soc_camera_desc sdesc = {.host_desc.bus_id = ici->nr,};
- char clk_name[V4L2_SUBDEV_NAME_SIZE];
+ char clk_name[V4L2_CLK_NAME_SIZE];
unsigned int i;
int ret;
@@ -1634,7 +1634,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
struct soc_camera_async_client *sasc;
struct soc_of_info *info;
struct i2c_client *client;
- char clk_name[V4L2_SUBDEV_NAME_SIZE + 32];
+ char clk_name[V4L2_CLK_NAME_SIZE];
int ret;
/* allocate a new subdev and add match info to it */
diff --git a/drivers/media/usb/em28xx/em28xx-camera.c
b/drivers/media/usb/em28xx/em28xx-camera.c
index ed0b3a8..121cdfc 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -322,7 +322,7 @@ int em28xx_detect_sensor(struct em28xx *dev)
int em28xx_init_camera(struct em28xx *dev)
{
- char clk_name[V4L2_SUBDEV_NAME_SIZE];
+ char clk_name[V4L2_CLK_NAME_SIZE];
struct i2c_client *client = >i2c_client[dev->def_i2c_bus];
struct i2c_adapter *adap = >i2c_adap[dev->def_i2c_bus];
struct em28xx_v4l2 *v4l2 = dev->v4l2;
diff --git a/include/media/v4l2-clk.h b/include/media/v4l2-clk.h
index 34891ea..2b94662 100644
--- a/include/media/v4l2-clk.h
+++ b/include/media/v4l2-clk.h
@@ -65,6 +65,8 @@ static inline struct v4l2_clk *v4l2_clk_register_fixed(const
char *dev_id,
return __v4l2_clk_register_fixed(dev_id, rate, THIS_MODULE);
}
+#define V4L2_CLK_NAME_SIZE 64
+
#define v4l2_clk_name_i2c(name, size, adap, client) snprintf(name, size, \
"%d-%04x", adap, client)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/4] soc_camera: get the clock name by using macro: v4l2_clk_name_i2c()
Since v4l2_clk_name_i2c() is defined, so just reuse it.
Signed-off-by: Josh Wu
---
drivers/media/platform/soc_camera/soc_camera.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index 9d24d44..d165bff 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1393,8 +1393,8 @@ static int soc_camera_i2c_init(struct soc_camera_device
*icd,
ssdd->sd_pdata.regulators = NULL;
shd->board_info->platform_data = ssdd;
- snprintf(clk_name, sizeof(clk_name), "%d-%04x",
-shd->i2c_adapter_id, shd->board_info->addr);
+ v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
+ shd->i2c_adapter_id, shd->board_info->addr);
icd->clk = v4l2_clk_register(_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd->clk)) {
@@ -1574,8 +1574,9 @@ static int scan_async_group(struct soc_camera_host *ici,
icd->sasc = sasc;
icd->parent = ici->v4l2_dev.dev;
- snprintf(clk_name, sizeof(clk_name), "%d-%04x",
-sasd->asd.match.i2c.adapter_id, sasd->asd.match.i2c.address);
+ v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
+ sasd->asd.match.i2c.adapter_id,
+ sasd->asd.match.i2c.address);
icd->clk = v4l2_clk_register(_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd->clk)) {
@@ -1676,8 +1677,8 @@ static int soc_of_bind(struct soc_camera_host *ici,
client = of_find_i2c_device_by_node(remote);
if (client)
- snprintf(clk_name, sizeof(clk_name), "%d-%04x",
-client->adapter->nr, client->addr);
+ v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
+ client->adapter->nr, client->addr);
else
snprintf(clk_name, sizeof(clk_name), "of-%s",
of_node_full_name(remote));
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/4] soc-camera: fix the bug which will fail to search the registered v4l2-clk
This patch set will fix a bug in soc-camera, which will fail to search the v4l2-clk if the i2c sensor is probed later than soc-camera host. It also add some clean up for v4l2-clk code and usage. Josh Wu (4): soc_camera: get the clock name by using macro: v4l2_clk_name_i2c() v4l2-clk: add new macro for v4l2_clk_name_of() v4l2-clk: add new definition: V4L2_CLK_NAME_SIZE v4l2-clk: v4l2_clk_get() also need to find the of_fullname clock drivers/media/platform/soc_camera/soc_camera.c | 23 --- drivers/media/usb/em28xx/em28xx-camera.c | 2 +- drivers/media/v4l2-core/v4l2-clk.c | 9 + include/media/v4l2-clk.h | 5 + 4 files changed, 27 insertions(+), 12 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/4] soc-camera: fix the bug which will fail to search the registered v4l2-clk
This patch set will fix a bug in soc-camera, which will fail to search the v4l2-clk if the i2c sensor is probed later than soc-camera host. It also add some clean up for v4l2-clk code and usage. Josh Wu (4): soc_camera: get the clock name by using macro: v4l2_clk_name_i2c() v4l2-clk: add new macro for v4l2_clk_name_of() v4l2-clk: add new definition: V4L2_CLK_NAME_SIZE v4l2-clk: v4l2_clk_get() also need to find the of_fullname clock drivers/media/platform/soc_camera/soc_camera.c | 23 --- drivers/media/usb/em28xx/em28xx-camera.c | 2 +- drivers/media/v4l2-core/v4l2-clk.c | 9 + include/media/v4l2-clk.h | 5 + 4 files changed, 27 insertions(+), 12 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/4] soc_camera: get the clock name by using macro: v4l2_clk_name_i2c()
Since v4l2_clk_name_i2c() is defined, so just reuse it.
Signed-off-by: Josh Wu <josh...@atmel.com>
---
drivers/media/platform/soc_camera/soc_camera.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index 9d24d44..d165bff 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1393,8 +1393,8 @@ static int soc_camera_i2c_init(struct soc_camera_device
*icd,
ssdd->sd_pdata.regulators = NULL;
shd->board_info->platform_data = ssdd;
- snprintf(clk_name, sizeof(clk_name), "%d-%04x",
-shd->i2c_adapter_id, shd->board_info->addr);
+ v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
+ shd->i2c_adapter_id, shd->board_info->addr);
icd->clk = v4l2_clk_register(_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd->clk)) {
@@ -1574,8 +1574,9 @@ static int scan_async_group(struct soc_camera_host *ici,
icd->sasc = sasc;
icd->parent = ici->v4l2_dev.dev;
- snprintf(clk_name, sizeof(clk_name), "%d-%04x",
-sasd->asd.match.i2c.adapter_id, sasd->asd.match.i2c.address);
+ v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
+ sasd->asd.match.i2c.adapter_id,
+ sasd->asd.match.i2c.address);
icd->clk = v4l2_clk_register(_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd->clk)) {
@@ -1676,8 +1677,8 @@ static int soc_of_bind(struct soc_camera_host *ici,
client = of_find_i2c_device_by_node(remote);
if (client)
- snprintf(clk_name, sizeof(clk_name), "%d-%04x",
-client->adapter->nr, client->addr);
+ v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
+ client->adapter->nr, client->addr);
else
snprintf(clk_name, sizeof(clk_name), "of-%s",
of_node_full_name(remote));
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/4] v4l2-clk: v4l2_clk_get() also need to find the of_fullname clock
The soc-camera host will be probed and register a v4l2_clk, but if on
that moment, the i2c device is not available, then the registered
v4l2_clk name is a OF string not a I2C string.
So when i2c sensor probed and call v4l2_clk_get(), it only search the
clock with I2C string, like "1-0030".
This patch will search the clock with OF string name if fail to find the
clock with I2C string name.
Signed-off-by: Josh Wu <josh...@atmel.com>
---
drivers/media/v4l2-core/v4l2-clk.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-clk.c
b/drivers/media/v4l2-core/v4l2-clk.c
index 34e416a..297e10e 100644
--- a/drivers/media/v4l2-core/v4l2-clk.c
+++ b/drivers/media/v4l2-core/v4l2-clk.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
@@ -39,6 +40,7 @@ struct v4l2_clk *v4l2_clk_get(struct device *dev, const char
*id)
{
struct v4l2_clk *clk;
struct clk *ccf_clk = clk_get(dev, id);
+ char clk_name[V4L2_CLK_NAME_SIZE];
if (PTR_ERR(ccf_clk) == -EPROBE_DEFER)
return ERR_PTR(-EPROBE_DEFER);
@@ -57,6 +59,13 @@ struct v4l2_clk *v4l2_clk_get(struct device *dev, const char
*id)
mutex_lock(_lock);
clk = v4l2_clk_find(dev_name(dev));
+ /* if dev_name is not found, try use the OF name to find again */
+ if (PTR_ERR(clk) == -ENODEV && dev->of_node) {
+ v4l2_clk_name_of(clk_name, sizeof(clk_name),
+of_node_full_name(dev->of_node));
+ clk = v4l2_clk_find(clk_name);
+ }
+
if (!IS_ERR(clk))
atomic_inc(>use_count);
mutex_unlock(_lock);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/4] v4l2-clk: add new macro for v4l2_clk_name_of()
This macro is used to generate a OF string for a v4l2 clock.
Signed-off-by: Josh Wu <josh...@atmel.com>
---
drivers/media/platform/soc_camera/soc_camera.c | 4 ++--
include/media/v4l2-clk.h | 3 +++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index d165bff..673f1d4 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1680,8 +1680,8 @@ static int soc_of_bind(struct soc_camera_host *ici,
v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
client->adapter->nr, client->addr);
else
- snprintf(clk_name, sizeof(clk_name), "of-%s",
-of_node_full_name(remote));
+ v4l2_clk_name_of(clk_name, sizeof(clk_name),
+of_node_full_name(remote));
icd->clk = v4l2_clk_register(_camera_clk_ops, clk_name, icd);
if (IS_ERR(icd->clk)) {
diff --git a/include/media/v4l2-clk.h b/include/media/v4l2-clk.h
index 3ef6e3d..34891ea 100644
--- a/include/media/v4l2-clk.h
+++ b/include/media/v4l2-clk.h
@@ -68,4 +68,7 @@ static inline struct v4l2_clk *v4l2_clk_register_fixed(const
char *dev_id,
#define v4l2_clk_name_i2c(name, size, adap, client) snprintf(name, size, \
"%d-%04x", adap, client)
+#define v4l2_clk_name_of(name, size, of_full_name) snprintf(name, size, \
+ "of-%s", of_full_name)
+
#endif
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/4] v4l2-clk: add new definition: V4L2_CLK_NAME_SIZE
Make all v4l2-clk's clock name use V4L2_CLK_NAME_SIZE definition.
In future, if the string increased we just need to change the
V4L2_CLK_NAME_SIZE once.
Signed-off-by: Josh Wu <josh...@atmel.com>
---
drivers/media/platform/soc_camera/soc_camera.c | 6 +++---
drivers/media/usb/em28xx/em28xx-camera.c | 2 +-
include/media/v4l2-clk.h | 2 ++
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index 673f1d4..506a569 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1362,7 +1362,7 @@ static int soc_camera_i2c_init(struct soc_camera_device
*icd,
struct soc_camera_host_desc *shd = >host_desc;
struct i2c_adapter *adap;
struct v4l2_subdev *subdev;
- char clk_name[V4L2_SUBDEV_NAME_SIZE];
+ char clk_name[V4L2_CLK_NAME_SIZE];
int ret;
/* First find out how we link the main client */
@@ -1528,7 +1528,7 @@ static int scan_async_group(struct soc_camera_host *ici,
struct soc_camera_async_client *sasc;
struct soc_camera_device *icd;
struct soc_camera_desc sdesc = {.host_desc.bus_id = ici->nr,};
- char clk_name[V4L2_SUBDEV_NAME_SIZE];
+ char clk_name[V4L2_CLK_NAME_SIZE];
unsigned int i;
int ret;
@@ -1634,7 +1634,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
struct soc_camera_async_client *sasc;
struct soc_of_info *info;
struct i2c_client *client;
- char clk_name[V4L2_SUBDEV_NAME_SIZE + 32];
+ char clk_name[V4L2_CLK_NAME_SIZE];
int ret;
/* allocate a new subdev and add match info to it */
diff --git a/drivers/media/usb/em28xx/em28xx-camera.c
b/drivers/media/usb/em28xx/em28xx-camera.c
index ed0b3a8..121cdfc 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -322,7 +322,7 @@ int em28xx_detect_sensor(struct em28xx *dev)
int em28xx_init_camera(struct em28xx *dev)
{
- char clk_name[V4L2_SUBDEV_NAME_SIZE];
+ char clk_name[V4L2_CLK_NAME_SIZE];
struct i2c_client *client = >i2c_client[dev->def_i2c_bus];
struct i2c_adapter *adap = >i2c_adap[dev->def_i2c_bus];
struct em28xx_v4l2 *v4l2 = dev->v4l2;
diff --git a/include/media/v4l2-clk.h b/include/media/v4l2-clk.h
index 34891ea..2b94662 100644
--- a/include/media/v4l2-clk.h
+++ b/include/media/v4l2-clk.h
@@ -65,6 +65,8 @@ static inline struct v4l2_clk *v4l2_clk_register_fixed(const
char *dev_id,
return __v4l2_clk_register_fixed(dev_id, rate, THIS_MODULE);
}
+#define V4L2_CLK_NAME_SIZE 64
+
#define v4l2_clk_name_i2c(name, size, adap, client) snprintf(name, size, \
"%d-%04x", adap, client)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: at91/dt: ov2640: add hsync/vsync-active property
Hi, Nicolas
On 9/18/2015 10:09 PM, Nicolas Ferre wrote:
Le 18/09/2015 13:28, Josh Wu a écrit :
On at91sam9x5ek/at91sam9m10g45ek/sama5d3xek boards, we use the parallel
connection for ov2640. So we must set the hsync/vsync property (1 means
active high).
Otherwise, the connection would be seen as BT.656 or BT.1120.
Signed-off-by: Josh Wu
Hi Josh,
Does this patch apply because of the new enhancement that you had
proposed in "media: atmel-isi: parse the DT parameters for
vsync/hsync/pixclock polarity" or does it apply even before?
Is it a fix that applies to older kernels?
It's is a fix, according to the dt binding of video interface.
So it should be applied before:
"media: atmel-isi: parse the DT parameters for
vsync/hsync/pixclock polarity".
it has no impact in older kernel as the driver doesn't parse that
property yet.
So, should I wait for the enhancements to enter media git tree or can I
take them anyway?
yes, you can take it without waiting for the media tree. Thanks.
Best Regards,
Josh Wu
Bye,
---
arch/arm/boot/dts/at91sam9m10g45ek.dts | 2 ++
arch/arm/boot/dts/at91sam9x5ek.dtsi| 2 ++
arch/arm/boot/dts/sama5d3xmb.dtsi | 2 ++
3 files changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/at91sam9m10g45ek.dts
b/arch/arm/boot/dts/at91sam9m10g45ek.dts
index d1ae60a..9d16ef8 100644
--- a/arch/arm/boot/dts/at91sam9m10g45ek.dts
+++ b/arch/arm/boot/dts/at91sam9m10g45ek.dts
@@ -198,6 +198,8 @@
isi_0: endpoint {
remote-endpoint = <_0>;
bus-width = <8>;
+ vsync-active = <1>;
+ hsync-active = <1>;
};
};
};
diff --git a/arch/arm/boot/dts/at91sam9x5ek.dtsi
b/arch/arm/boot/dts/at91sam9x5ek.dtsi
index d237c46..479f200 100644
--- a/arch/arm/boot/dts/at91sam9x5ek.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5ek.dtsi
@@ -66,6 +66,8 @@
isi_0: endpoint@0 {
remote-endpoint = <_0>;
bus-width = <8>;
+ vsync-active = <1>;
+ hsync-active = <1>;
};
};
};
diff --git a/arch/arm/boot/dts/sama5d3xmb.dtsi
b/arch/arm/boot/dts/sama5d3xmb.dtsi
index 83bee7a..8901042 100644
--- a/arch/arm/boot/dts/sama5d3xmb.dtsi
+++ b/arch/arm/boot/dts/sama5d3xmb.dtsi
@@ -87,6 +87,8 @@
isi_0: endpoint {
remote-endpoint = <_0>;
bus-width = <8>;
+ vsync-active = <1>;
+ hsync-active = <1>;
};
};
};
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: at91/dt: ov2640: add hsync/vsync-active property
Hi, Nicolas
On 9/18/2015 10:09 PM, Nicolas Ferre wrote:
Le 18/09/2015 13:28, Josh Wu a écrit :
On at91sam9x5ek/at91sam9m10g45ek/sama5d3xek boards, we use the parallel
connection for ov2640. So we must set the hsync/vsync property (1 means
active high).
Otherwise, the connection would be seen as BT.656 or BT.1120.
Signed-off-by: Josh Wu <josh...@atmel.com>
Hi Josh,
Does this patch apply because of the new enhancement that you had
proposed in "media: atmel-isi: parse the DT parameters for
vsync/hsync/pixclock polarity" or does it apply even before?
Is it a fix that applies to older kernels?
It's is a fix, according to the dt binding of video interface.
So it should be applied before:
"media: atmel-isi: parse the DT parameters for
vsync/hsync/pixclock polarity".
it has no impact in older kernel as the driver doesn't parse that
property yet.
So, should I wait for the enhancements to enter media git tree or can I
take them anyway?
yes, you can take it without waiting for the media tree. Thanks.
Best Regards,
Josh Wu
Bye,
---
arch/arm/boot/dts/at91sam9m10g45ek.dts | 2 ++
arch/arm/boot/dts/at91sam9x5ek.dtsi| 2 ++
arch/arm/boot/dts/sama5d3xmb.dtsi | 2 ++
3 files changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/at91sam9m10g45ek.dts
b/arch/arm/boot/dts/at91sam9m10g45ek.dts
index d1ae60a..9d16ef8 100644
--- a/arch/arm/boot/dts/at91sam9m10g45ek.dts
+++ b/arch/arm/boot/dts/at91sam9m10g45ek.dts
@@ -198,6 +198,8 @@
isi_0: endpoint {
remote-endpoint = <_0>;
bus-width = <8>;
+ vsync-active = <1>;
+ hsync-active = <1>;
};
};
};
diff --git a/arch/arm/boot/dts/at91sam9x5ek.dtsi
b/arch/arm/boot/dts/at91sam9x5ek.dtsi
index d237c46..479f200 100644
--- a/arch/arm/boot/dts/at91sam9x5ek.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5ek.dtsi
@@ -66,6 +66,8 @@
isi_0: endpoint@0 {
remote-endpoint = <_0>;
bus-width = <8>;
+ vsync-active = <1>;
+ hsync-active = <1>;
};
};
};
diff --git a/arch/arm/boot/dts/sama5d3xmb.dtsi
b/arch/arm/boot/dts/sama5d3xmb.dtsi
index 83bee7a..8901042 100644
--- a/arch/arm/boot/dts/sama5d3xmb.dtsi
+++ b/arch/arm/boot/dts/sama5d3xmb.dtsi
@@ -87,6 +87,8 @@
isi_0: endpoint {
remote-endpoint = <_0>;
bus-width = <8>;
+ vsync-active = <1>;
+ hsync-active = <1>;
};
};
};
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ARM: at91/dt: ov2640: add hsync/vsync-active property
On at91sam9x5ek/at91sam9m10g45ek/sama5d3xek boards, we use the parallel
connection for ov2640. So we must set the hsync/vsync property (1 means
active high).
Otherwise, the connection would be seen as BT.656 or BT.1120.
Signed-off-by: Josh Wu
---
arch/arm/boot/dts/at91sam9m10g45ek.dts | 2 ++
arch/arm/boot/dts/at91sam9x5ek.dtsi| 2 ++
arch/arm/boot/dts/sama5d3xmb.dtsi | 2 ++
3 files changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/at91sam9m10g45ek.dts
b/arch/arm/boot/dts/at91sam9m10g45ek.dts
index d1ae60a..9d16ef8 100644
--- a/arch/arm/boot/dts/at91sam9m10g45ek.dts
+++ b/arch/arm/boot/dts/at91sam9m10g45ek.dts
@@ -198,6 +198,8 @@
isi_0: endpoint {
remote-endpoint = <_0>;
bus-width = <8>;
+ vsync-active = <1>;
+ hsync-active = <1>;
};
};
};
diff --git a/arch/arm/boot/dts/at91sam9x5ek.dtsi
b/arch/arm/boot/dts/at91sam9x5ek.dtsi
index d237c46..479f200 100644
--- a/arch/arm/boot/dts/at91sam9x5ek.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5ek.dtsi
@@ -66,6 +66,8 @@
isi_0: endpoint@0 {
remote-endpoint = <_0>;
bus-width = <8>;
+ vsync-active = <1>;
+ hsync-active = <1>;
};
};
};
diff --git a/arch/arm/boot/dts/sama5d3xmb.dtsi
b/arch/arm/boot/dts/sama5d3xmb.dtsi
index 83bee7a..8901042 100644
--- a/arch/arm/boot/dts/sama5d3xmb.dtsi
+++ b/arch/arm/boot/dts/sama5d3xmb.dtsi
@@ -87,6 +87,8 @@
isi_0: endpoint {
remote-endpoint = <_0>;
bus-width = <8>;
+ vsync-active = <1>;
+ hsync-active = <1>;
};
};
};
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] media: soc-camera: increase the length of clk_name on soc_of_bind()
Hi, Guennadi On 8/30/2015 10:06 PM, Guennadi Liakhovetski wrote: Hi Josh, Sorry, I missed the 4.3 merge cycle, but isn't this patch a fix? Isn't it fixing soc-camera / atmel-isi on a specific platform, where the clock name is longer, than currently supported? Is this platform in the mainline and its current camera support is broken because of this? I missed your email, so sorry for the late reply. yes, it will break the detect flow if the i2c camera is loaded as module. In such a case we could still push it in for 4.3 So it is a fix, it is great if this one can still go into 4.3. Best Regards, Josh Wu Thanks Guennadi On Tue, 4 Aug 2015, Josh Wu wrote: Since in soc_of_bind() it may use the of node's full name as the clk_name, and this full name may be longer than 32 characters, take at91 i2c sensor as an example, length is 34 bytes: /ahb/apb/i2c@f8028000/camera@0x30 So this patch increase the clk_name[] array size to 64. It seems big enough so far. Signed-off-by: Josh Wu --- drivers/media/platform/soc_camera/soc_camera.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c index d708df4..fcf3e97 100644 --- a/drivers/media/platform/soc_camera/soc_camera.c +++ b/drivers/media/platform/soc_camera/soc_camera.c @@ -1621,7 +1621,7 @@ static int soc_of_bind(struct soc_camera_host *ici, struct soc_camera_async_client *sasc; struct soc_of_info *info; struct i2c_client *client; - char clk_name[V4L2_SUBDEV_NAME_SIZE]; + char clk_name[V4L2_SUBDEV_NAME_SIZE + 32]; int ret; /* allocate a new subdev and add match info to it */ -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ARM: at91/dt: ov2640: add hsync/vsync-active property
On at91sam9x5ek/at91sam9m10g45ek/sama5d3xek boards, we use the parallel
connection for ov2640. So we must set the hsync/vsync property (1 means
active high).
Otherwise, the connection would be seen as BT.656 or BT.1120.
Signed-off-by: Josh Wu <josh...@atmel.com>
---
arch/arm/boot/dts/at91sam9m10g45ek.dts | 2 ++
arch/arm/boot/dts/at91sam9x5ek.dtsi| 2 ++
arch/arm/boot/dts/sama5d3xmb.dtsi | 2 ++
3 files changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/at91sam9m10g45ek.dts
b/arch/arm/boot/dts/at91sam9m10g45ek.dts
index d1ae60a..9d16ef8 100644
--- a/arch/arm/boot/dts/at91sam9m10g45ek.dts
+++ b/arch/arm/boot/dts/at91sam9m10g45ek.dts
@@ -198,6 +198,8 @@
isi_0: endpoint {
remote-endpoint = <_0>;
bus-width = <8>;
+ vsync-active = <1>;
+ hsync-active = <1>;
};
};
};
diff --git a/arch/arm/boot/dts/at91sam9x5ek.dtsi
b/arch/arm/boot/dts/at91sam9x5ek.dtsi
index d237c46..479f200 100644
--- a/arch/arm/boot/dts/at91sam9x5ek.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5ek.dtsi
@@ -66,6 +66,8 @@
isi_0: endpoint@0 {
remote-endpoint = <_0>;
bus-width = <8>;
+ vsync-active = <1>;
+ hsync-active = <1>;
};
};
};
diff --git a/arch/arm/boot/dts/sama5d3xmb.dtsi
b/arch/arm/boot/dts/sama5d3xmb.dtsi
index 83bee7a..8901042 100644
--- a/arch/arm/boot/dts/sama5d3xmb.dtsi
+++ b/arch/arm/boot/dts/sama5d3xmb.dtsi
@@ -87,6 +87,8 @@
isi_0: endpoint {
remote-endpoint = <_0>;
bus-width = <8>;
+ vsync-active = <1>;
+ hsync-active = <1>;
};
};
};
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] media: soc-camera: increase the length of clk_name on soc_of_bind()
Hi, Guennadi On 8/30/2015 10:06 PM, Guennadi Liakhovetski wrote: Hi Josh, Sorry, I missed the 4.3 merge cycle, but isn't this patch a fix? Isn't it fixing soc-camera / atmel-isi on a specific platform, where the clock name is longer, than currently supported? Is this platform in the mainline and its current camera support is broken because of this? I missed your email, so sorry for the late reply. yes, it will break the detect flow if the i2c camera is loaded as module. In such a case we could still push it in for 4.3 So it is a fix, it is great if this one can still go into 4.3. Best Regards, Josh Wu Thanks Guennadi On Tue, 4 Aug 2015, Josh Wu wrote: Since in soc_of_bind() it may use the of node's full name as the clk_name, and this full name may be longer than 32 characters, take at91 i2c sensor as an example, length is 34 bytes: /ahb/apb/i2c@f8028000/camera@0x30 So this patch increase the clk_name[] array size to 64. It seems big enough so far. Signed-off-by: Josh Wu <josh...@atmel.com> --- drivers/media/platform/soc_camera/soc_camera.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c index d708df4..fcf3e97 100644 --- a/drivers/media/platform/soc_camera/soc_camera.c +++ b/drivers/media/platform/soc_camera/soc_camera.c @@ -1621,7 +1621,7 @@ static int soc_of_bind(struct soc_camera_host *ici, struct soc_camera_async_client *sasc; struct soc_of_info *info; struct i2c_client *client; - char clk_name[V4L2_SUBDEV_NAME_SIZE]; + char clk_name[V4L2_SUBDEV_NAME_SIZE + 32]; int ret; /* allocate a new subdev and add match info to it */ -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [media] atmel-isi: Protect PM-only functions to kill warning
Hi, Geert Uytterhoeven
Thanks for the patch.
On 9/6/2015 6:08 PM, Geert Uytterhoeven wrote:
If CONFIG_PM=n:
drivers/media/platform/soc_camera/atmel-isi.c:1044: warning:
‘atmel_isi_runtime_suspend’ defined but not used
drivers/media/platform/soc_camera/atmel-isi.c:1054: warning:
‘atmel_isi_runtime_resume’ defined but not used
Protect the unused functions by #ifdef CONFIG_PM to fix this.
Signed-off-by: Geert Uytterhoeven
Acked-by: Josh Wu
Best Regards,
Josh Wu
---
Resend with correct suject
---
drivers/media/platform/soc_camera/atmel-isi.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 90701726a06a2e5c..ccf30ccbe389233f 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -1040,6 +1040,7 @@ err_alloc_ctx:
return ret;
}
+#ifdef CONFIG_PM
static int atmel_isi_runtime_suspend(struct device *dev)
{
struct soc_camera_host *soc_host = to_soc_camera_host(dev);
@@ -1058,6 +1059,7 @@ static int atmel_isi_runtime_resume(struct device *dev)
return clk_prepare_enable(isi->pclk);
}
+#endif /* CONFIG_PM */
static const struct dev_pm_ops atmel_isi_dev_pm_ops = {
SET_RUNTIME_PM_OPS(atmel_isi_runtime_suspend,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [media] atmel-isi: Protect PM-only functions to kill warning
Hi, Geert Uytterhoeven
Thanks for the patch.
On 9/6/2015 6:08 PM, Geert Uytterhoeven wrote:
If CONFIG_PM=n:
drivers/media/platform/soc_camera/atmel-isi.c:1044: warning:
‘atmel_isi_runtime_suspend’ defined but not used
drivers/media/platform/soc_camera/atmel-isi.c:1054: warning:
‘atmel_isi_runtime_resume’ defined but not used
Protect the unused functions by #ifdef CONFIG_PM to fix this.
Signed-off-by: Geert Uytterhoeven <ge...@linux-m68k.org>
Acked-by: Josh Wu <josh...@atmel.com>
Best Regards,
Josh Wu
---
Resend with correct suject
---
drivers/media/platform/soc_camera/atmel-isi.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 90701726a06a2e5c..ccf30ccbe389233f 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -1040,6 +1040,7 @@ err_alloc_ctx:
return ret;
}
+#ifdef CONFIG_PM
static int atmel_isi_runtime_suspend(struct device *dev)
{
struct soc_camera_host *soc_host = to_soc_camera_host(dev);
@@ -1058,6 +1059,7 @@ static int atmel_isi_runtime_resume(struct device *dev)
return clk_prepare_enable(isi->pclk);
}
+#endif /* CONFIG_PM */
static const struct dev_pm_ops atmel_isi_dev_pm_ops = {
SET_RUNTIME_PM_OPS(atmel_isi_runtime_suspend,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] media: atmel-isi: setup the ISI_CFG2 register directly
Hi, Guennadi
Thanks for the review.
On 8/30/2015 4:48 PM, Guennadi Liakhovetski wrote:
Hi Josh,
On Wed, 17 Jun 2015, Josh Wu wrote:
In the function configure_geometry(), we will setup the ISI CFG2
according to the sensor output format.
It make no sense to just read back the CFG2 register and just set part
of it.
So just set up this register directly makes things simpler.
Simpler doesn't necessarily mean better or more correct:) There are other
fields in that register and currently the driver preserves them, with this
patch you overwrite them with 0. 0 happens to be the reset value of that
register. So, you should at least mention that in this patch description,
just saying "simpler" doesn't convince me.
Correct, I should mention that the reset value (0) of cfg2 means the YUV
mode in the commit message.
To use YUV mode we need to clear COL_SPACE (bit 15 of CFG2) and since
the reset value is 0, so in the code, I didn't need do anything.
So, at least I'd modify that, I
can do that myself. But in general I'm not even sure why this patch is
needed. Yes, currently those fields of that register are unused, so, we
can assume they stay at their reset values. But firstly the hardware can
change and at some point the reset value can change, or those other fields
will get set indirectly by something. Or the driver will change at some
point to support more fields of that register and then this code will have
to be changed again.
I understand your concern.
maybe a better solution is explicitly set the COL_SPACE (bit 15) to 0
for the YUV formats. like:
#define ISI_CFG2_COL_SPACE_YUV(0 << 15)
case MEDIA_BUS_FMT_YVYU8_2X8:
cfg2 = ISI_CFG2_COL_SPACE_YUV | ISI_CFG2_YCC_SWAP_MODE_1;
break;
And above modifications can be sent with RGB format support patch in future.
So, I'd ask you again - do you really want this
patch?
yes, this patch is needed. And in future i will add the RGB format settings.
If you insist - I'll take it, but I'd add the "reset value"
reasoning.
That would be great, thank you very much.
Best Regards,
Josh Wu
Otherwise maybe just drop it?
Thanks
Guennadi
Currently only support YUV format from camera sensor.
Signed-off-by: Josh Wu
---
drivers/media/platform/soc_camera/atmel-isi.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 9070172..8bc40ca 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -105,24 +105,25 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
static int configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
- u32 cfg2, cr;
+ u32 cfg2;
+ /* According to sensor's output format to set cfg2 */
switch (code) {
/* YUV, including grey */
case MEDIA_BUS_FMT_Y8_1X8:
- cr = ISI_CFG2_GRAYSCALE;
+ cfg2 = ISI_CFG2_GRAYSCALE;
break;
case MEDIA_BUS_FMT_VYUY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_3;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
case MEDIA_BUS_FMT_UYVY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_2;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_2;
break;
case MEDIA_BUS_FMT_YVYU8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_1;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_1;
break;
case MEDIA_BUS_FMT_YUYV8_2X8:
- cr = ISI_CFG2_YCC_SWAP_DEFAULT;
+ cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
default:
@@ -130,17 +131,10 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
-
- cfg2 = isi_readl(isi, ISI_CFG2);
- /* Set YCC swap mode */
- cfg2 &= ~ISI_CFG2_YCC_SWAP_MODE_MASK;
- cfg2 |= cr;
/* Set width */
- cfg2 &= ~(ISI_CFG2_IM_HSIZE_MASK);
cfg2 |= ((width - 1) << ISI_CFG2_IM_HSIZE_OFFSET) &
ISI_CFG2_IM_HSIZE_MASK;
/* Set height */
- cfg2 &= ~(ISI_CFG2_IM_VSIZE_MASK);
cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
& ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] media: atmel-isi: setup the ISI_CFG2 register directly
Hi, Guennadi
Thanks for the review.
On 8/30/2015 4:48 PM, Guennadi Liakhovetski wrote:
Hi Josh,
On Wed, 17 Jun 2015, Josh Wu wrote:
In the function configure_geometry(), we will setup the ISI CFG2
according to the sensor output format.
It make no sense to just read back the CFG2 register and just set part
of it.
So just set up this register directly makes things simpler.
Simpler doesn't necessarily mean better or more correct:) There are other
fields in that register and currently the driver preserves them, with this
patch you overwrite them with 0. 0 happens to be the reset value of that
register. So, you should at least mention that in this patch description,
just saying "simpler" doesn't convince me.
Correct, I should mention that the reset value (0) of cfg2 means the YUV
mode in the commit message.
To use YUV mode we need to clear COL_SPACE (bit 15 of CFG2) and since
the reset value is 0, so in the code, I didn't need do anything.
So, at least I'd modify that, I
can do that myself. But in general I'm not even sure why this patch is
needed. Yes, currently those fields of that register are unused, so, we
can assume they stay at their reset values. But firstly the hardware can
change and at some point the reset value can change, or those other fields
will get set indirectly by something. Or the driver will change at some
point to support more fields of that register and then this code will have
to be changed again.
I understand your concern.
maybe a better solution is explicitly set the COL_SPACE (bit 15) to 0
for the YUV formats. like:
#define ISI_CFG2_COL_SPACE_YUV(0 << 15)
case MEDIA_BUS_FMT_YVYU8_2X8:
cfg2 = ISI_CFG2_COL_SPACE_YUV | ISI_CFG2_YCC_SWAP_MODE_1;
break;
And above modifications can be sent with RGB format support patch in future.
So, I'd ask you again - do you really want this
patch?
yes, this patch is needed. And in future i will add the RGB format settings.
If you insist - I'll take it, but I'd add the "reset value"
reasoning.
That would be great, thank you very much.
Best Regards,
Josh Wu
Otherwise maybe just drop it?
Thanks
Guennadi
Currently only support YUV format from camera sensor.
Signed-off-by: Josh Wu <josh...@atmel.com>
---
drivers/media/platform/soc_camera/atmel-isi.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 9070172..8bc40ca 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -105,24 +105,25 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
static int configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
- u32 cfg2, cr;
+ u32 cfg2;
+ /* According to sensor's output format to set cfg2 */
switch (code) {
/* YUV, including grey */
case MEDIA_BUS_FMT_Y8_1X8:
- cr = ISI_CFG2_GRAYSCALE;
+ cfg2 = ISI_CFG2_GRAYSCALE;
break;
case MEDIA_BUS_FMT_VYUY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_3;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
case MEDIA_BUS_FMT_UYVY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_2;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_2;
break;
case MEDIA_BUS_FMT_YVYU8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_1;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_1;
break;
case MEDIA_BUS_FMT_YUYV8_2X8:
- cr = ISI_CFG2_YCC_SWAP_DEFAULT;
+ cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
default:
@@ -130,17 +131,10 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
-
- cfg2 = isi_readl(isi, ISI_CFG2);
- /* Set YCC swap mode */
- cfg2 &= ~ISI_CFG2_YCC_SWAP_MODE_MASK;
- cfg2 |= cr;
/* Set width */
- cfg2 &= ~(ISI_CFG2_IM_HSIZE_MASK);
cfg2 |= ((width - 1) << ISI_CFG2_IM_HSIZE_OFFSET) &
ISI_CFG2_IM_HSIZE_MASK;
/* Set height */
- cfg2 &= ~(ISI_CFG2_IM_VSIZE_MASK);
cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
& ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 3/3] media: atmel-isi: add sanity check for supported formats in try/set_fmt()
Hi, Laurent
On 8/22/2015 2:22 AM, Laurent Pinchart wrote:
Hi Josh,
Thank you for the patch.
On Friday 21 August 2015 16:08:14 Josh Wu wrote:
After adding the format check in try_fmt()/set_fmt(), we don't need any
format check in configure_geometry(). So make configure_geometry() as
void type.
Signed-off-by: Josh Wu
---
Changes in v3:
- check the whether format is supported, if no then return a default
format.
- misc changes according to Laurent's feedback.
Changes in v2:
- new added patch
drivers/media/platform/soc_camera/atmel-isi.c | 37 ++--
1 file changed, 29 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c index fe9247a..84c91d3
100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -102,17 +102,19 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
return readl(isi->regs + reg);
}
-static int configure_geometry(struct atmel_isi *isi, u32 width,
+static void configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
u32 cfg2;
/* According to sensor's output format to set cfg2 */
switch (code) {
- /* YUV, including grey */
+ default:
+ /* Grey */
case MEDIA_BUS_FMT_Y8_1X8:
cfg2 = ISI_CFG2_GRAYSCALE;
break;
+ /* YUV */
case MEDIA_BUS_FMT_VYUY8_2X8:
cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
@@ -126,8 +128,6 @@ static int configure_geometry(struct atmel_isi *isi, u32
width, cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
- default:
- return -EINVAL;
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
@@ -138,8 +138,23 @@ static int configure_geometry(struct atmel_isi *isi,
u32 width, cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
& ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
+}
- return 0;
+static bool is_supported(struct soc_camera_device *icd,
+ const u32 pixformat)
+{
+ switch (pixformat) {
+ /* YUV, including grey */
+ case V4L2_PIX_FMT_GREY:
+ case V4L2_PIX_FMT_YUYV:
+ case V4L2_PIX_FMT_UYVY:
+ case V4L2_PIX_FMT_YVYU:
+ case V4L2_PIX_FMT_VYUY:
+ return true;
+ /* RGB, TODO */
+ default:
+ return false;
+ }
}
static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
@@ -390,10 +405,8 @@ static int start_streaming(struct vb2_queue *vq,
unsigned int count) /* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
- ret = configure_geometry(isi, icd->user_width, icd->user_height,
+ configure_geometry(isi, icd->user_width, icd->user_height,
icd->current_fmt->code);
- if (ret < 0)
- return ret;
spin_lock_irq(>lock);
/* Clear any pending interrupt */
@@ -491,6 +504,10 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd, struct v4l2_mbus_framefmt *mf =
int ret;
+ /* check with atmel-isi support format, if not support use UYVY */
+ if (!is_supported(icd, pix->pixelformat))
+ pix->pixelformat = V4L2_PIX_FMT_YUYV;
The comment mentions UYVY and the code uses YUYV.
Oops, forgotten to change the comments. I'll fix it.
+
xlate = soc_camera_xlate_by_fourcc(icd, pix->pixelformat);
if (!xlate) {
dev_warn(icd->parent, "Format %x not found\n",
Can this still happen ?
I think this warning should not happen if user call set_fmt() with the
format that get from by get_formats().
But it is a common pattern in soc_camera host code to handle this error.
@@ -540,6 +557,10 @@ static int isi_camera_try_fmt(struct soc_camera_device
*icd, u32 pixfmt = pix->pixelformat;
int ret;
+ /* check with atmel-isi support format, if not support use UYVY */
+ if (!is_supported(icd, pix->pixelformat))
+ pix->pixelformat = V4L2_PIX_FMT_YUYV;
+
xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
if (pixfmt && !xlate) {
dev_warn(icd->parent, "Format %x not found\n", pixfmt);
Same comment here.
I wonder whether most of the content of isi_camera_set_fmt() and
isi_camera_try_fmt() could be factorized out into a shared function.
I agree. the current set_fmt() doesn't touch any hardware, so it's
almost same as try_fmt().
Best Regards,
Josh Wu
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 3/3] media: atmel-isi: add sanity check for supported formats in try/set_fmt()
Hi, Laurent
On 8/22/2015 2:22 AM, Laurent Pinchart wrote:
Hi Josh,
Thank you for the patch.
On Friday 21 August 2015 16:08:14 Josh Wu wrote:
After adding the format check in try_fmt()/set_fmt(), we don't need any
format check in configure_geometry(). So make configure_geometry() as
void type.
Signed-off-by: Josh Wu josh...@atmel.com
---
Changes in v3:
- check the whether format is supported, if no then return a default
format.
- misc changes according to Laurent's feedback.
Changes in v2:
- new added patch
drivers/media/platform/soc_camera/atmel-isi.c | 37 ++--
1 file changed, 29 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c index fe9247a..84c91d3
100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -102,17 +102,19 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
return readl(isi-regs + reg);
}
-static int configure_geometry(struct atmel_isi *isi, u32 width,
+static void configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
u32 cfg2;
/* According to sensor's output format to set cfg2 */
switch (code) {
- /* YUV, including grey */
+ default:
+ /* Grey */
case MEDIA_BUS_FMT_Y8_1X8:
cfg2 = ISI_CFG2_GRAYSCALE;
break;
+ /* YUV */
case MEDIA_BUS_FMT_VYUY8_2X8:
cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
@@ -126,8 +128,6 @@ static int configure_geometry(struct atmel_isi *isi, u32
width, cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
- default:
- return -EINVAL;
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
@@ -138,8 +138,23 @@ static int configure_geometry(struct atmel_isi *isi,
u32 width, cfg2 |= ((height - 1) ISI_CFG2_IM_VSIZE_OFFSET)
ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
+}
- return 0;
+static bool is_supported(struct soc_camera_device *icd,
+ const u32 pixformat)
+{
+ switch (pixformat) {
+ /* YUV, including grey */
+ case V4L2_PIX_FMT_GREY:
+ case V4L2_PIX_FMT_YUYV:
+ case V4L2_PIX_FMT_UYVY:
+ case V4L2_PIX_FMT_YVYU:
+ case V4L2_PIX_FMT_VYUY:
+ return true;
+ /* RGB, TODO */
+ default:
+ return false;
+ }
}
static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
@@ -390,10 +405,8 @@ static int start_streaming(struct vb2_queue *vq,
unsigned int count) /* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
- ret = configure_geometry(isi, icd-user_width, icd-user_height,
+ configure_geometry(isi, icd-user_width, icd-user_height,
icd-current_fmt-code);
- if (ret 0)
- return ret;
spin_lock_irq(isi-lock);
/* Clear any pending interrupt */
@@ -491,6 +504,10 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd, struct v4l2_mbus_framefmt *mf = format.format;
int ret;
+ /* check with atmel-isi support format, if not support use UYVY */
+ if (!is_supported(icd, pix-pixelformat))
+ pix-pixelformat = V4L2_PIX_FMT_YUYV;
The comment mentions UYVY and the code uses YUYV.
Oops, forgotten to change the comments. I'll fix it.
+
xlate = soc_camera_xlate_by_fourcc(icd, pix-pixelformat);
if (!xlate) {
dev_warn(icd-parent, Format %x not found\n,
Can this still happen ?
I think this warning should not happen if user call set_fmt() with the
format that get from by get_formats().
But it is a common pattern in soc_camera host code to handle this error.
@@ -540,6 +557,10 @@ static int isi_camera_try_fmt(struct soc_camera_device
*icd, u32 pixfmt = pix-pixelformat;
int ret;
+ /* check with atmel-isi support format, if not support use UYVY */
+ if (!is_supported(icd, pix-pixelformat))
+ pix-pixelformat = V4L2_PIX_FMT_YUYV;
+
xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
if (pixfmt !xlate) {
dev_warn(icd-parent, Format %x not found\n, pixfmt);
Same comment here.
I wonder whether most of the content of isi_camera_set_fmt() and
isi_camera_try_fmt() could be factorized out into a shared function.
I agree. the current set_fmt() doesn't touch any hardware, so it's
almost same as try_fmt().
Best Regards,
Josh Wu
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 3/3] media: atmel-isi: add sanity check for supported formats in try/set_fmt()
After adding the format check in try_fmt()/set_fmt(), we don't need any
format check in configure_geometry(). So make configure_geometry() as
void type.
Signed-off-by: Josh Wu
---
Changes in v3:
- check the whether format is supported, if no then return a default
format.
- misc changes according to Laurent's feedback.
Changes in v2:
- new added patch
drivers/media/platform/soc_camera/atmel-isi.c | 37 +--
1 file changed, 29 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index fe9247a..84c91d3 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -102,17 +102,19 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
return readl(isi->regs + reg);
}
-static int configure_geometry(struct atmel_isi *isi, u32 width,
+static void configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
u32 cfg2;
/* According to sensor's output format to set cfg2 */
switch (code) {
- /* YUV, including grey */
+ default:
+ /* Grey */
case MEDIA_BUS_FMT_Y8_1X8:
cfg2 = ISI_CFG2_GRAYSCALE;
break;
+ /* YUV */
case MEDIA_BUS_FMT_VYUY8_2X8:
cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
@@ -126,8 +128,6 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
- default:
- return -EINVAL;
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
@@ -138,8 +138,23 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
& ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
+}
- return 0;
+static bool is_supported(struct soc_camera_device *icd,
+ const u32 pixformat)
+{
+ switch (pixformat) {
+ /* YUV, including grey */
+ case V4L2_PIX_FMT_GREY:
+ case V4L2_PIX_FMT_YUYV:
+ case V4L2_PIX_FMT_UYVY:
+ case V4L2_PIX_FMT_YVYU:
+ case V4L2_PIX_FMT_VYUY:
+ return true;
+ /* RGB, TODO */
+ default:
+ return false;
+ }
}
static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
@@ -390,10 +405,8 @@ static int start_streaming(struct vb2_queue *vq, unsigned
int count)
/* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
- ret = configure_geometry(isi, icd->user_width, icd->user_height,
+ configure_geometry(isi, icd->user_width, icd->user_height,
icd->current_fmt->code);
- if (ret < 0)
- return ret;
spin_lock_irq(>lock);
/* Clear any pending interrupt */
@@ -491,6 +504,10 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd,
struct v4l2_mbus_framefmt *mf =
int ret;
+ /* check with atmel-isi support format, if not support use UYVY */
+ if (!is_supported(icd, pix->pixelformat))
+ pix->pixelformat = V4L2_PIX_FMT_YUYV;
+
xlate = soc_camera_xlate_by_fourcc(icd, pix->pixelformat);
if (!xlate) {
dev_warn(icd->parent, "Format %x not found\n",
@@ -540,6 +557,10 @@ static int isi_camera_try_fmt(struct soc_camera_device
*icd,
u32 pixfmt = pix->pixelformat;
int ret;
+ /* check with atmel-isi support format, if not support use UYVY */
+ if (!is_supported(icd, pix->pixelformat))
+ pix->pixelformat = V4L2_PIX_FMT_YUYV;
+
xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
if (pixfmt && !xlate) {
dev_warn(icd->parent, "Format %x not found\n", pixfmt);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 2/3] media: atmel-isi: move configure_geometry() to start_streaming()
As in set_fmt() function we only need to know which format is been set,
we don't need to access the ISI hardware in this moment.
So move the configure_geometry(), which access the ISI hardware, to
start_streaming() will make code more consistent and simpler.
Signed-off-by: Josh Wu
Reviewed-by: Laurent Pinchart
---
Changes in v3: None
Changes in v2:
- Add Laurent's reviewed-by tag.
drivers/media/platform/soc_camera/atmel-isi.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index b67da70..fe9247a 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -390,6 +390,11 @@ static int start_streaming(struct vb2_queue *vq, unsigned
int count)
/* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
+ ret = configure_geometry(isi, icd->user_width, icd->user_height,
+ icd->current_fmt->code);
+ if (ret < 0)
+ return ret;
+
spin_lock_irq(>lock);
/* Clear any pending interrupt */
isi_readl(isi, ISI_STATUS);
@@ -477,8 +482,6 @@ static int isi_camera_init_videobuf(struct vb2_queue *q,
static int isi_camera_set_fmt(struct soc_camera_device *icd,
struct v4l2_format *f)
{
- struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
- struct atmel_isi *isi = ici->priv;
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
const struct soc_camera_format_xlate *xlate;
struct v4l2_pix_format *pix = >fmt.pix;
@@ -511,16 +514,6 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd,
if (mf->code != xlate->code)
return -EINVAL;
- /* Enable PM and peripheral clock before operate isi registers */
- pm_runtime_get_sync(ici->v4l2_dev.dev);
-
- ret = configure_geometry(isi, pix->width, pix->height, xlate->code);
-
- pm_runtime_put(ici->v4l2_dev.dev);
-
- if (ret < 0)
- return ret;
-
pix->width = mf->width;
pix->height = mf->height;
pix->field = mf->field;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/3] media: atmel-isi: setup the ISI_CFG2 register directly
In the function configure_geometry(), we will setup the ISI CFG2
according to the sensor output format.
It make no sense to just read back the CFG2 register and just set part
of it.
So just set up this register directly makes things simpler.
Currently only support YUV format from camera sensor.
Signed-off-by: Josh Wu
Reviewed-by: Laurent Pinchart
---
Changes in v3: None
Changes in v2:
- add Laurent's reviewed-by tag.
drivers/media/platform/soc_camera/atmel-isi.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index f39132c..b67da70 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -105,24 +105,25 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
static int configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
- u32 cfg2, cr;
+ u32 cfg2;
+ /* According to sensor's output format to set cfg2 */
switch (code) {
/* YUV, including grey */
case MEDIA_BUS_FMT_Y8_1X8:
- cr = ISI_CFG2_GRAYSCALE;
+ cfg2 = ISI_CFG2_GRAYSCALE;
break;
case MEDIA_BUS_FMT_VYUY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_3;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
case MEDIA_BUS_FMT_UYVY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_2;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_2;
break;
case MEDIA_BUS_FMT_YVYU8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_1;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_1;
break;
case MEDIA_BUS_FMT_YUYV8_2X8:
- cr = ISI_CFG2_YCC_SWAP_DEFAULT;
+ cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
default:
@@ -130,17 +131,10 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
-
- cfg2 = isi_readl(isi, ISI_CFG2);
- /* Set YCC swap mode */
- cfg2 &= ~ISI_CFG2_YCC_SWAP_MODE_MASK;
- cfg2 |= cr;
/* Set width */
- cfg2 &= ~(ISI_CFG2_IM_HSIZE_MASK);
cfg2 |= ((width - 1) << ISI_CFG2_IM_HSIZE_OFFSET) &
ISI_CFG2_IM_HSIZE_MASK;
/* Set height */
- cfg2 &= ~(ISI_CFG2_IM_VSIZE_MASK);
cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
& ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 3/3] media: atmel-isi: add sanity check for supported formats in try/set_fmt()
After adding the format check in try_fmt()/set_fmt(), we don't need any
format check in configure_geometry(). So make configure_geometry() as
void type.
Signed-off-by: Josh Wu josh...@atmel.com
---
Changes in v3:
- check the whether format is supported, if no then return a default
format.
- misc changes according to Laurent's feedback.
Changes in v2:
- new added patch
drivers/media/platform/soc_camera/atmel-isi.c | 37 +--
1 file changed, 29 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index fe9247a..84c91d3 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -102,17 +102,19 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
return readl(isi-regs + reg);
}
-static int configure_geometry(struct atmel_isi *isi, u32 width,
+static void configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
u32 cfg2;
/* According to sensor's output format to set cfg2 */
switch (code) {
- /* YUV, including grey */
+ default:
+ /* Grey */
case MEDIA_BUS_FMT_Y8_1X8:
cfg2 = ISI_CFG2_GRAYSCALE;
break;
+ /* YUV */
case MEDIA_BUS_FMT_VYUY8_2X8:
cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
@@ -126,8 +128,6 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
- default:
- return -EINVAL;
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
@@ -138,8 +138,23 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
cfg2 |= ((height - 1) ISI_CFG2_IM_VSIZE_OFFSET)
ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
+}
- return 0;
+static bool is_supported(struct soc_camera_device *icd,
+ const u32 pixformat)
+{
+ switch (pixformat) {
+ /* YUV, including grey */
+ case V4L2_PIX_FMT_GREY:
+ case V4L2_PIX_FMT_YUYV:
+ case V4L2_PIX_FMT_UYVY:
+ case V4L2_PIX_FMT_YVYU:
+ case V4L2_PIX_FMT_VYUY:
+ return true;
+ /* RGB, TODO */
+ default:
+ return false;
+ }
}
static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
@@ -390,10 +405,8 @@ static int start_streaming(struct vb2_queue *vq, unsigned
int count)
/* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
- ret = configure_geometry(isi, icd-user_width, icd-user_height,
+ configure_geometry(isi, icd-user_width, icd-user_height,
icd-current_fmt-code);
- if (ret 0)
- return ret;
spin_lock_irq(isi-lock);
/* Clear any pending interrupt */
@@ -491,6 +504,10 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd,
struct v4l2_mbus_framefmt *mf = format.format;
int ret;
+ /* check with atmel-isi support format, if not support use UYVY */
+ if (!is_supported(icd, pix-pixelformat))
+ pix-pixelformat = V4L2_PIX_FMT_YUYV;
+
xlate = soc_camera_xlate_by_fourcc(icd, pix-pixelformat);
if (!xlate) {
dev_warn(icd-parent, Format %x not found\n,
@@ -540,6 +557,10 @@ static int isi_camera_try_fmt(struct soc_camera_device
*icd,
u32 pixfmt = pix-pixelformat;
int ret;
+ /* check with atmel-isi support format, if not support use UYVY */
+ if (!is_supported(icd, pix-pixelformat))
+ pix-pixelformat = V4L2_PIX_FMT_YUYV;
+
xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
if (pixfmt !xlate) {
dev_warn(icd-parent, Format %x not found\n, pixfmt);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 2/3] media: atmel-isi: move configure_geometry() to start_streaming()
As in set_fmt() function we only need to know which format is been set,
we don't need to access the ISI hardware in this moment.
So move the configure_geometry(), which access the ISI hardware, to
start_streaming() will make code more consistent and simpler.
Signed-off-by: Josh Wu josh...@atmel.com
Reviewed-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
Changes in v3: None
Changes in v2:
- Add Laurent's reviewed-by tag.
drivers/media/platform/soc_camera/atmel-isi.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index b67da70..fe9247a 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -390,6 +390,11 @@ static int start_streaming(struct vb2_queue *vq, unsigned
int count)
/* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
+ ret = configure_geometry(isi, icd-user_width, icd-user_height,
+ icd-current_fmt-code);
+ if (ret 0)
+ return ret;
+
spin_lock_irq(isi-lock);
/* Clear any pending interrupt */
isi_readl(isi, ISI_STATUS);
@@ -477,8 +482,6 @@ static int isi_camera_init_videobuf(struct vb2_queue *q,
static int isi_camera_set_fmt(struct soc_camera_device *icd,
struct v4l2_format *f)
{
- struct soc_camera_host *ici = to_soc_camera_host(icd-parent);
- struct atmel_isi *isi = ici-priv;
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
const struct soc_camera_format_xlate *xlate;
struct v4l2_pix_format *pix = f-fmt.pix;
@@ -511,16 +514,6 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd,
if (mf-code != xlate-code)
return -EINVAL;
- /* Enable PM and peripheral clock before operate isi registers */
- pm_runtime_get_sync(ici-v4l2_dev.dev);
-
- ret = configure_geometry(isi, pix-width, pix-height, xlate-code);
-
- pm_runtime_put(ici-v4l2_dev.dev);
-
- if (ret 0)
- return ret;
-
pix-width = mf-width;
pix-height = mf-height;
pix-field = mf-field;
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/3] media: atmel-isi: setup the ISI_CFG2 register directly
In the function configure_geometry(), we will setup the ISI CFG2
according to the sensor output format.
It make no sense to just read back the CFG2 register and just set part
of it.
So just set up this register directly makes things simpler.
Currently only support YUV format from camera sensor.
Signed-off-by: Josh Wu josh...@atmel.com
Reviewed-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
Changes in v3: None
Changes in v2:
- add Laurent's reviewed-by tag.
drivers/media/platform/soc_camera/atmel-isi.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index f39132c..b67da70 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -105,24 +105,25 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
static int configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
- u32 cfg2, cr;
+ u32 cfg2;
+ /* According to sensor's output format to set cfg2 */
switch (code) {
/* YUV, including grey */
case MEDIA_BUS_FMT_Y8_1X8:
- cr = ISI_CFG2_GRAYSCALE;
+ cfg2 = ISI_CFG2_GRAYSCALE;
break;
case MEDIA_BUS_FMT_VYUY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_3;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
case MEDIA_BUS_FMT_UYVY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_2;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_2;
break;
case MEDIA_BUS_FMT_YVYU8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_1;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_1;
break;
case MEDIA_BUS_FMT_YUYV8_2X8:
- cr = ISI_CFG2_YCC_SWAP_DEFAULT;
+ cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
default:
@@ -130,17 +131,10 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
-
- cfg2 = isi_readl(isi, ISI_CFG2);
- /* Set YCC swap mode */
- cfg2 = ~ISI_CFG2_YCC_SWAP_MODE_MASK;
- cfg2 |= cr;
/* Set width */
- cfg2 = ~(ISI_CFG2_IM_HSIZE_MASK);
cfg2 |= ((width - 1) ISI_CFG2_IM_HSIZE_OFFSET)
ISI_CFG2_IM_HSIZE_MASK;
/* Set height */
- cfg2 = ~(ISI_CFG2_IM_VSIZE_MASK);
cfg2 |= ((height - 1) ISI_CFG2_IM_VSIZE_OFFSET)
ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/3] media: atmel-isi: add sanity check for supported formats in set_fmt()
Hi, Laurent
Thanks for the review.
On 8/21/2015 2:30 AM, Laurent Pinchart wrote:
Hi Josh,
Thank you for the patch.
On Wednesday 05 August 2015 11:26:29 Josh Wu wrote:
After adding the format check in set_fmt(), we don't need any format check
in configure_geometry(). So make configure_geometry() as void type.
Signed-off-by: Josh Wu
---
Changes in v2:
- new added patch
drivers/media/platform/soc_camera/atmel-isi.c | 39 ++--
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c index cb46aec..d0df518
100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -103,17 +103,19 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
return readl(isi->regs + reg);
}
-static int configure_geometry(struct atmel_isi *isi, u32 width,
+static void configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
u32 cfg2;
/* According to sensor's output format to set cfg2 */
switch (code) {
- /* YUV, including grey */
+ default:
+ /* Grey */
case MEDIA_BUS_FMT_Y8_1X8:
cfg2 = ISI_CFG2_GRAYSCALE;
break;
+ /* YUV */
case MEDIA_BUS_FMT_VYUY8_2X8:
cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
@@ -127,8 +129,6 @@ static int configure_geometry(struct atmel_isi *isi, u32
width, cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
- default:
- return -EINVAL;
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
@@ -139,8 +139,29 @@ static int configure_geometry(struct atmel_isi *isi,
u32 width, cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
& ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
+}
- return 0;
+static bool is_supported(struct soc_camera_device *icd,
+ const struct soc_camera_format_xlate *xlate)
+{
+ bool ret = true;
+
+ switch (xlate->code) {
+ /* YUV, including grey */
+ case MEDIA_BUS_FMT_Y8_1X8:
+ case MEDIA_BUS_FMT_VYUY8_2X8:
+ case MEDIA_BUS_FMT_UYVY8_2X8:
+ case MEDIA_BUS_FMT_YVYU8_2X8:
+ case MEDIA_BUS_FMT_YUYV8_2X8:
I would just return true here and false below, and remove the ret variable.
Ok.
+ break;
+ /* RGB, TODO */
+ default:
+ dev_err(icd->parent, "not supported format: %d\n",
+ xlate->code);
If this can happen when userspace asks for an unsupported format I don't think
you should print an error message to the kernel log.
ok, I will remove this.
+ ret = false;
+ }
+
+ return ret;
}
static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
@@ -391,10 +412,8 @@ static int start_streaming(struct vb2_queue *vq,
unsigned int count) /* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
- ret = configure_geometry(isi, icd->user_width, icd->user_height,
+ configure_geometry(isi, icd->user_width, icd->user_height,
icd->current_fmt->code);
- if (ret < 0)
- return ret;
spin_lock_irq(>lock);
/* Clear any pending interrupt */
@@ -515,6 +534,10 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd, if (mf->code != xlate->code)
return -EINVAL;
+ /* check with atmel-isi support format */
+ if (!is_supported(icd, xlate))
+ return -EINVAL;
+
S_FMT is supposed to pick a suitable default format when the requested format
isn't supported. It shouldn't return an error.
So I will move this check to beginning of S_FMT and if format not
support, I will select a default format for it.
Best Regards,
Josh Wu
pix->width = mf->width;
pix->height = mf->height;
pix->field = mf->field;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/3] media: atmel-isi: add sanity check for supported formats in set_fmt()
Hi, Laurent
Could you have time to review this patch? so that I can send a pull
request with your reviewed-by tags.
Best Regards,
Josh Wu
On 8/5/2015 11:26 AM, Josh Wu wrote:
After adding the format check in set_fmt(), we don't need any format check
in configure_geometry(). So make configure_geometry() as void type.
Signed-off-by: Josh Wu
---
Changes in v2:
- new added patch
drivers/media/platform/soc_camera/atmel-isi.c | 39 +--
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index cb46aec..d0df518 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -103,17 +103,19 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
return readl(isi->regs + reg);
}
-static int configure_geometry(struct atmel_isi *isi, u32 width,
+static void configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
u32 cfg2;
/* According to sensor's output format to set cfg2 */
switch (code) {
- /* YUV, including grey */
+ default:
+ /* Grey */
case MEDIA_BUS_FMT_Y8_1X8:
cfg2 = ISI_CFG2_GRAYSCALE;
break;
+ /* YUV */
case MEDIA_BUS_FMT_VYUY8_2X8:
cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
@@ -127,8 +129,6 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
- default:
- return -EINVAL;
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
@@ -139,8 +139,29 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
& ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
+}
- return 0;
+static bool is_supported(struct soc_camera_device *icd,
+ const struct soc_camera_format_xlate *xlate)
+{
+ bool ret = true;
+
+ switch (xlate->code) {
+ /* YUV, including grey */
+ case MEDIA_BUS_FMT_Y8_1X8:
+ case MEDIA_BUS_FMT_VYUY8_2X8:
+ case MEDIA_BUS_FMT_UYVY8_2X8:
+ case MEDIA_BUS_FMT_YVYU8_2X8:
+ case MEDIA_BUS_FMT_YUYV8_2X8:
+ break;
+ /* RGB, TODO */
+ default:
+ dev_err(icd->parent, "not supported format: %d\n",
+ xlate->code);
+ ret = false;
+ }
+
+ return ret;
}
static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
@@ -391,10 +412,8 @@ static int start_streaming(struct vb2_queue *vq, unsigned
int count)
/* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
- ret = configure_geometry(isi, icd->user_width, icd->user_height,
+ configure_geometry(isi, icd->user_width, icd->user_height,
icd->current_fmt->code);
- if (ret < 0)
- return ret;
spin_lock_irq(>lock);
/* Clear any pending interrupt */
@@ -515,6 +534,10 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd,
if (mf->code != xlate->code)
return -EINVAL;
+ /* check with atmel-isi support format */
+ if (!is_supported(icd, xlate))
+ return -EINVAL;
+
pix->width = mf->width;
pix->height = mf->height;
pix->field = mf->field;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/3] media: atmel-isi: add sanity check for supported formats in set_fmt()
Hi, Laurent
Could you have time to review this patch? so that I can send a pull
request with your reviewed-by tags.
Best Regards,
Josh Wu
On 8/5/2015 11:26 AM, Josh Wu wrote:
After adding the format check in set_fmt(), we don't need any format check
in configure_geometry(). So make configure_geometry() as void type.
Signed-off-by: Josh Wu josh...@atmel.com
---
Changes in v2:
- new added patch
drivers/media/platform/soc_camera/atmel-isi.c | 39 +--
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index cb46aec..d0df518 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -103,17 +103,19 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
return readl(isi-regs + reg);
}
-static int configure_geometry(struct atmel_isi *isi, u32 width,
+static void configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
u32 cfg2;
/* According to sensor's output format to set cfg2 */
switch (code) {
- /* YUV, including grey */
+ default:
+ /* Grey */
case MEDIA_BUS_FMT_Y8_1X8:
cfg2 = ISI_CFG2_GRAYSCALE;
break;
+ /* YUV */
case MEDIA_BUS_FMT_VYUY8_2X8:
cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
@@ -127,8 +129,6 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
- default:
- return -EINVAL;
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
@@ -139,8 +139,29 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
cfg2 |= ((height - 1) ISI_CFG2_IM_VSIZE_OFFSET)
ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
+}
- return 0;
+static bool is_supported(struct soc_camera_device *icd,
+ const struct soc_camera_format_xlate *xlate)
+{
+ bool ret = true;
+
+ switch (xlate-code) {
+ /* YUV, including grey */
+ case MEDIA_BUS_FMT_Y8_1X8:
+ case MEDIA_BUS_FMT_VYUY8_2X8:
+ case MEDIA_BUS_FMT_UYVY8_2X8:
+ case MEDIA_BUS_FMT_YVYU8_2X8:
+ case MEDIA_BUS_FMT_YUYV8_2X8:
+ break;
+ /* RGB, TODO */
+ default:
+ dev_err(icd-parent, not supported format: %d\n,
+ xlate-code);
+ ret = false;
+ }
+
+ return ret;
}
static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
@@ -391,10 +412,8 @@ static int start_streaming(struct vb2_queue *vq, unsigned
int count)
/* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
- ret = configure_geometry(isi, icd-user_width, icd-user_height,
+ configure_geometry(isi, icd-user_width, icd-user_height,
icd-current_fmt-code);
- if (ret 0)
- return ret;
spin_lock_irq(isi-lock);
/* Clear any pending interrupt */
@@ -515,6 +534,10 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd,
if (mf-code != xlate-code)
return -EINVAL;
+ /* check with atmel-isi support format */
+ if (!is_supported(icd, xlate))
+ return -EINVAL;
+
pix-width = mf-width;
pix-height = mf-height;
pix-field = mf-field;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/3] media: atmel-isi: add sanity check for supported formats in set_fmt()
Hi, Laurent
Thanks for the review.
On 8/21/2015 2:30 AM, Laurent Pinchart wrote:
Hi Josh,
Thank you for the patch.
On Wednesday 05 August 2015 11:26:29 Josh Wu wrote:
After adding the format check in set_fmt(), we don't need any format check
in configure_geometry(). So make configure_geometry() as void type.
Signed-off-by: Josh Wu josh...@atmel.com
---
Changes in v2:
- new added patch
drivers/media/platform/soc_camera/atmel-isi.c | 39 ++--
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c index cb46aec..d0df518
100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -103,17 +103,19 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
return readl(isi-regs + reg);
}
-static int configure_geometry(struct atmel_isi *isi, u32 width,
+static void configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
u32 cfg2;
/* According to sensor's output format to set cfg2 */
switch (code) {
- /* YUV, including grey */
+ default:
+ /* Grey */
case MEDIA_BUS_FMT_Y8_1X8:
cfg2 = ISI_CFG2_GRAYSCALE;
break;
+ /* YUV */
case MEDIA_BUS_FMT_VYUY8_2X8:
cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
@@ -127,8 +129,6 @@ static int configure_geometry(struct atmel_isi *isi, u32
width, cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
- default:
- return -EINVAL;
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
@@ -139,8 +139,29 @@ static int configure_geometry(struct atmel_isi *isi,
u32 width, cfg2 |= ((height - 1) ISI_CFG2_IM_VSIZE_OFFSET)
ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
+}
- return 0;
+static bool is_supported(struct soc_camera_device *icd,
+ const struct soc_camera_format_xlate *xlate)
+{
+ bool ret = true;
+
+ switch (xlate-code) {
+ /* YUV, including grey */
+ case MEDIA_BUS_FMT_Y8_1X8:
+ case MEDIA_BUS_FMT_VYUY8_2X8:
+ case MEDIA_BUS_FMT_UYVY8_2X8:
+ case MEDIA_BUS_FMT_YVYU8_2X8:
+ case MEDIA_BUS_FMT_YUYV8_2X8:
I would just return true here and false below, and remove the ret variable.
Ok.
+ break;
+ /* RGB, TODO */
+ default:
+ dev_err(icd-parent, not supported format: %d\n,
+ xlate-code);
If this can happen when userspace asks for an unsupported format I don't think
you should print an error message to the kernel log.
ok, I will remove this.
+ ret = false;
+ }
+
+ return ret;
}
static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
@@ -391,10 +412,8 @@ static int start_streaming(struct vb2_queue *vq,
unsigned int count) /* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
- ret = configure_geometry(isi, icd-user_width, icd-user_height,
+ configure_geometry(isi, icd-user_width, icd-user_height,
icd-current_fmt-code);
- if (ret 0)
- return ret;
spin_lock_irq(isi-lock);
/* Clear any pending interrupt */
@@ -515,6 +534,10 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd, if (mf-code != xlate-code)
return -EINVAL;
+ /* check with atmel-isi support format */
+ if (!is_supported(icd, xlate))
+ return -EINVAL;
+
S_FMT is supposed to pick a suitable default format when the requested format
isn't supported. It shouldn't return an error.
So I will move this check to beginning of S_FMT and if format not
support, I will select a default format for it.
Best Regards,
Josh Wu
pix-width = mf-width;
pix-height = mf-height;
pix-field = mf-field;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/3] media: atmel-isi: move configure_geometry() to start_streaming()
As in set_fmt() function we only need to know which format is been set,
we don't need to access the ISI hardware in this moment.
So move the configure_geometry(), which access the ISI hardware, to
start_streaming() will make code more consistent and simpler.
Signed-off-by: Josh Wu
Reviewed-by: Laurent Pinchart
---
Changes in v2:
- Add Laurent's reviewed-by tag.
drivers/media/platform/soc_camera/atmel-isi.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 0fd6bc9..cb46aec 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -391,6 +391,11 @@ static int start_streaming(struct vb2_queue *vq, unsigned
int count)
/* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
+ ret = configure_geometry(isi, icd->user_width, icd->user_height,
+ icd->current_fmt->code);
+ if (ret < 0)
+ return ret;
+
spin_lock_irq(>lock);
/* Clear any pending interrupt */
isi_readl(isi, ISI_STATUS);
@@ -478,8 +483,6 @@ static int isi_camera_init_videobuf(struct vb2_queue *q,
static int isi_camera_set_fmt(struct soc_camera_device *icd,
struct v4l2_format *f)
{
- struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
- struct atmel_isi *isi = ici->priv;
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
const struct soc_camera_format_xlate *xlate;
struct v4l2_pix_format *pix = >fmt.pix;
@@ -512,16 +515,6 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd,
if (mf->code != xlate->code)
return -EINVAL;
- /* Enable PM and peripheral clock before operate isi registers */
- pm_runtime_get_sync(ici->v4l2_dev.dev);
-
- ret = configure_geometry(isi, pix->width, pix->height, xlate->code);
-
- pm_runtime_put(ici->v4l2_dev.dev);
-
- if (ret < 0)
- return ret;
-
pix->width = mf->width;
pix->height = mf->height;
pix->field = mf->field;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 3/3] media: atmel-isi: add sanity check for supported formats in set_fmt()
After adding the format check in set_fmt(), we don't need any format check
in configure_geometry(). So make configure_geometry() as void type.
Signed-off-by: Josh Wu
---
Changes in v2:
- new added patch
drivers/media/platform/soc_camera/atmel-isi.c | 39 +--
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index cb46aec..d0df518 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -103,17 +103,19 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
return readl(isi->regs + reg);
}
-static int configure_geometry(struct atmel_isi *isi, u32 width,
+static void configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
u32 cfg2;
/* According to sensor's output format to set cfg2 */
switch (code) {
- /* YUV, including grey */
+ default:
+ /* Grey */
case MEDIA_BUS_FMT_Y8_1X8:
cfg2 = ISI_CFG2_GRAYSCALE;
break;
+ /* YUV */
case MEDIA_BUS_FMT_VYUY8_2X8:
cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
@@ -127,8 +129,6 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
- default:
- return -EINVAL;
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
@@ -139,8 +139,29 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
& ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
+}
- return 0;
+static bool is_supported(struct soc_camera_device *icd,
+ const struct soc_camera_format_xlate *xlate)
+{
+ bool ret = true;
+
+ switch (xlate->code) {
+ /* YUV, including grey */
+ case MEDIA_BUS_FMT_Y8_1X8:
+ case MEDIA_BUS_FMT_VYUY8_2X8:
+ case MEDIA_BUS_FMT_UYVY8_2X8:
+ case MEDIA_BUS_FMT_YVYU8_2X8:
+ case MEDIA_BUS_FMT_YUYV8_2X8:
+ break;
+ /* RGB, TODO */
+ default:
+ dev_err(icd->parent, "not supported format: %d\n",
+ xlate->code);
+ ret = false;
+ }
+
+ return ret;
}
static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
@@ -391,10 +412,8 @@ static int start_streaming(struct vb2_queue *vq, unsigned
int count)
/* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
- ret = configure_geometry(isi, icd->user_width, icd->user_height,
+ configure_geometry(isi, icd->user_width, icd->user_height,
icd->current_fmt->code);
- if (ret < 0)
- return ret;
spin_lock_irq(>lock);
/* Clear any pending interrupt */
@@ -515,6 +534,10 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd,
if (mf->code != xlate->code)
return -EINVAL;
+ /* check with atmel-isi support format */
+ if (!is_supported(icd, xlate))
+ return -EINVAL;
+
pix->width = mf->width;
pix->height = mf->height;
pix->field = mf->field;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/3] media: atmel-isi: setup the ISI_CFG2 register directly
In the function configure_geometry(), we will setup the ISI CFG2
according to the sensor output format.
It make no sense to just read back the CFG2 register and just set part
of it.
So just set up this register directly makes things simpler.
Currently only support YUV format from camera sensor.
Signed-off-by: Josh Wu
Reviewed-by: Laurent Pinchart
---
Changes in v2:
- add Laurent's reviewed-by tag.
drivers/media/platform/soc_camera/atmel-isi.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 274a6f7..0fd6bc9 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -106,24 +106,25 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
static int configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
- u32 cfg2, cr;
+ u32 cfg2;
+ /* According to sensor's output format to set cfg2 */
switch (code) {
/* YUV, including grey */
case MEDIA_BUS_FMT_Y8_1X8:
- cr = ISI_CFG2_GRAYSCALE;
+ cfg2 = ISI_CFG2_GRAYSCALE;
break;
case MEDIA_BUS_FMT_VYUY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_3;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
case MEDIA_BUS_FMT_UYVY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_2;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_2;
break;
case MEDIA_BUS_FMT_YVYU8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_1;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_1;
break;
case MEDIA_BUS_FMT_YUYV8_2X8:
- cr = ISI_CFG2_YCC_SWAP_DEFAULT;
+ cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
default:
@@ -131,17 +132,10 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
-
- cfg2 = isi_readl(isi, ISI_CFG2);
- /* Set YCC swap mode */
- cfg2 &= ~ISI_CFG2_YCC_SWAP_MODE_MASK;
- cfg2 |= cr;
/* Set width */
- cfg2 &= ~(ISI_CFG2_IM_HSIZE_MASK);
cfg2 |= ((width - 1) << ISI_CFG2_IM_HSIZE_OFFSET) &
ISI_CFG2_IM_HSIZE_MASK;
/* Set height */
- cfg2 &= ~(ISI_CFG2_IM_VSIZE_MASK);
cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
& ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] media: soc-camera: increase the length of clk_name on soc_of_bind()
Since in soc_of_bind() it may use the of node's full name as the clk_name, and this full name may be longer than 32 characters, take at91 i2c sensor as an example, length is 34 bytes: /ahb/apb/i2c@f8028000/camera@0x30 So this patch increase the clk_name[] array size to 64. It seems big enough so far. Signed-off-by: Josh Wu --- drivers/media/platform/soc_camera/soc_camera.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c index d708df4..fcf3e97 100644 --- a/drivers/media/platform/soc_camera/soc_camera.c +++ b/drivers/media/platform/soc_camera/soc_camera.c @@ -1621,7 +1621,7 @@ static int soc_of_bind(struct soc_camera_host *ici, struct soc_camera_async_client *sasc; struct soc_of_info *info; struct i2c_client *client; - char clk_name[V4L2_SUBDEV_NAME_SIZE]; + char clk_name[V4L2_SUBDEV_NAME_SIZE + 32]; int ret; /* allocate a new subdev and add match info to it */ -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] media: atmel-isi: move configure_geometry() to start_streaming()
Hi, Laurent
On 8/3/2015 9:27 PM, Laurent Pinchart wrote:
Hi Josh,
On Monday 03 August 2015 11:56:01 Josh Wu wrote:
On 7/31/2015 10:37 PM, Laurent Pinchart wrote:
On Wednesday 17 June 2015 18:39:39 Josh Wu wrote:
As in set_fmt() function we only need to know which format is been set,
we don't need to access the ISI hardware in this moment.
So move the configure_geometry(), which access the ISI hardware, to
start_streaming() will make code more consistent and simpler.
Signed-off-by: Josh Wu
---
drivers/media/platform/soc_camera/atmel-isi.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c index 8bc40ca..b01086d
100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -390,6 +390,11 @@ static int start_streaming(struct vb2_queue *vq,
unsigned int count) /* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
+ ret = configure_geometry(isi, icd->user_width, icd->user_height,
+ icd->current_fmt->code);
I would also make configure_geometry a void function, as the only failure
case really can't occur.
I think this case can be reached if user require a RGB565 format to
capture and sensor also support RGB565 format.
As atmel-isi driver will provide RGB565 support via the pass-through
mode (maybe we need re-consider this part).
So that will cause the configure_geometry() returns an error since it
found the bus format is not Y8 or YUV422.
In my opinion, we should not change configure_geometry()'s return type,
until we add a insanity format check before we call configure_geometry()
in future.
It will really confuse the user if S_FMT accepts a format but STREAMON fails
due to the format being unsupported. Could that be fixed by defaulting to a
known supported format in S_FMT if the requested format isn't support ?
yes, it's the right way to go.
You
could then remove the error check in configure_geometry().
So I will send a v2 patches, which will add one more patch to add
insanity check on the S_FMT and remove the error check code in
configure_geometry().
And for this patch in v2, I will add your reviewed-by tag. Is that Okay
for you?
Best Regards,
Josh Wu
Apart from that,
Reviewed-by: Laurent Pinchart
Thanks for the review.
Best Regards,
Josh Wu
+ if (ret < 0)
+ return ret;
+
spin_lock_irq(>lock);
/* Clear any pending interrupt */
isi_readl(isi, ISI_STATUS);
@@ -477,8 +482,6 @@ static int isi_camera_init_videobuf(struct vb2_queue
*q, static int isi_camera_set_fmt(struct soc_camera_device *icd,
struct v4l2_format *f)
{
- struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
- struct atmel_isi *isi = ici->priv;
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
const struct soc_camera_format_xlate *xlate;
struct v4l2_pix_format *pix = >fmt.pix;
@@ -511,16 +514,6 @@ static int isi_camera_set_fmt(struct
soc_camera_device
*icd, if (mf->code != xlate->code)
return -EINVAL;
- /* Enable PM and peripheral clock before operate isi registers */
- pm_runtime_get_sync(ici->v4l2_dev.dev);
-
- ret = configure_geometry(isi, pix->width, pix->height, xlate->code);
-
- pm_runtime_put(ici->v4l2_dev.dev);
-
- if (ret < 0)
- return ret;
-
pix->width = mf->width;
pix->height = mf->height;
pix->field = mf->field;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] media: soc-camera: increase the length of clk_name on soc_of_bind()
Since in soc_of_bind() it may use the of node's full name as the clk_name, and this full name may be longer than 32 characters, take at91 i2c sensor as an example, length is 34 bytes: /ahb/apb/i2c@f8028000/camera@0x30 So this patch increase the clk_name[] array size to 64. It seems big enough so far. Signed-off-by: Josh Wu josh...@atmel.com --- drivers/media/platform/soc_camera/soc_camera.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c index d708df4..fcf3e97 100644 --- a/drivers/media/platform/soc_camera/soc_camera.c +++ b/drivers/media/platform/soc_camera/soc_camera.c @@ -1621,7 +1621,7 @@ static int soc_of_bind(struct soc_camera_host *ici, struct soc_camera_async_client *sasc; struct soc_of_info *info; struct i2c_client *client; - char clk_name[V4L2_SUBDEV_NAME_SIZE]; + char clk_name[V4L2_SUBDEV_NAME_SIZE + 32]; int ret; /* allocate a new subdev and add match info to it */ -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] media: atmel-isi: move configure_geometry() to start_streaming()
Hi, Laurent
On 8/3/2015 9:27 PM, Laurent Pinchart wrote:
Hi Josh,
On Monday 03 August 2015 11:56:01 Josh Wu wrote:
On 7/31/2015 10:37 PM, Laurent Pinchart wrote:
On Wednesday 17 June 2015 18:39:39 Josh Wu wrote:
As in set_fmt() function we only need to know which format is been set,
we don't need to access the ISI hardware in this moment.
So move the configure_geometry(), which access the ISI hardware, to
start_streaming() will make code more consistent and simpler.
Signed-off-by: Josh Wu josh...@atmel.com
---
drivers/media/platform/soc_camera/atmel-isi.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c index 8bc40ca..b01086d
100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -390,6 +390,11 @@ static int start_streaming(struct vb2_queue *vq,
unsigned int count) /* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
+ ret = configure_geometry(isi, icd-user_width, icd-user_height,
+ icd-current_fmt-code);
I would also make configure_geometry a void function, as the only failure
case really can't occur.
I think this case can be reached if user require a RGB565 format to
capture and sensor also support RGB565 format.
As atmel-isi driver will provide RGB565 support via the pass-through
mode (maybe we need re-consider this part).
So that will cause the configure_geometry() returns an error since it
found the bus format is not Y8 or YUV422.
In my opinion, we should not change configure_geometry()'s return type,
until we add a insanity format check before we call configure_geometry()
in future.
It will really confuse the user if S_FMT accepts a format but STREAMON fails
due to the format being unsupported. Could that be fixed by defaulting to a
known supported format in S_FMT if the requested format isn't support ?
yes, it's the right way to go.
You
could then remove the error check in configure_geometry().
So I will send a v2 patches, which will add one more patch to add
insanity check on the S_FMT and remove the error check code in
configure_geometry().
And for this patch in v2, I will add your reviewed-by tag. Is that Okay
for you?
Best Regards,
Josh Wu
Apart from that,
Reviewed-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Thanks for the review.
Best Regards,
Josh Wu
+ if (ret 0)
+ return ret;
+
spin_lock_irq(isi-lock);
/* Clear any pending interrupt */
isi_readl(isi, ISI_STATUS);
@@ -477,8 +482,6 @@ static int isi_camera_init_videobuf(struct vb2_queue
*q, static int isi_camera_set_fmt(struct soc_camera_device *icd,
struct v4l2_format *f)
{
- struct soc_camera_host *ici = to_soc_camera_host(icd-parent);
- struct atmel_isi *isi = ici-priv;
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
const struct soc_camera_format_xlate *xlate;
struct v4l2_pix_format *pix = f-fmt.pix;
@@ -511,16 +514,6 @@ static int isi_camera_set_fmt(struct
soc_camera_device
*icd, if (mf-code != xlate-code)
return -EINVAL;
- /* Enable PM and peripheral clock before operate isi registers */
- pm_runtime_get_sync(ici-v4l2_dev.dev);
-
- ret = configure_geometry(isi, pix-width, pix-height, xlate-code);
-
- pm_runtime_put(ici-v4l2_dev.dev);
-
- if (ret 0)
- return ret;
-
pix-width = mf-width;
pix-height = mf-height;
pix-field = mf-field;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/3] media: atmel-isi: move configure_geometry() to start_streaming()
As in set_fmt() function we only need to know which format is been set,
we don't need to access the ISI hardware in this moment.
So move the configure_geometry(), which access the ISI hardware, to
start_streaming() will make code more consistent and simpler.
Signed-off-by: Josh Wu josh...@atmel.com
Reviewed-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
Changes in v2:
- Add Laurent's reviewed-by tag.
drivers/media/platform/soc_camera/atmel-isi.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 0fd6bc9..cb46aec 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -391,6 +391,11 @@ static int start_streaming(struct vb2_queue *vq, unsigned
int count)
/* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
+ ret = configure_geometry(isi, icd-user_width, icd-user_height,
+ icd-current_fmt-code);
+ if (ret 0)
+ return ret;
+
spin_lock_irq(isi-lock);
/* Clear any pending interrupt */
isi_readl(isi, ISI_STATUS);
@@ -478,8 +483,6 @@ static int isi_camera_init_videobuf(struct vb2_queue *q,
static int isi_camera_set_fmt(struct soc_camera_device *icd,
struct v4l2_format *f)
{
- struct soc_camera_host *ici = to_soc_camera_host(icd-parent);
- struct atmel_isi *isi = ici-priv;
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
const struct soc_camera_format_xlate *xlate;
struct v4l2_pix_format *pix = f-fmt.pix;
@@ -512,16 +515,6 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd,
if (mf-code != xlate-code)
return -EINVAL;
- /* Enable PM and peripheral clock before operate isi registers */
- pm_runtime_get_sync(ici-v4l2_dev.dev);
-
- ret = configure_geometry(isi, pix-width, pix-height, xlate-code);
-
- pm_runtime_put(ici-v4l2_dev.dev);
-
- if (ret 0)
- return ret;
-
pix-width = mf-width;
pix-height = mf-height;
pix-field = mf-field;
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 3/3] media: atmel-isi: add sanity check for supported formats in set_fmt()
After adding the format check in set_fmt(), we don't need any format check
in configure_geometry(). So make configure_geometry() as void type.
Signed-off-by: Josh Wu josh...@atmel.com
---
Changes in v2:
- new added patch
drivers/media/platform/soc_camera/atmel-isi.c | 39 +--
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index cb46aec..d0df518 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -103,17 +103,19 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
return readl(isi-regs + reg);
}
-static int configure_geometry(struct atmel_isi *isi, u32 width,
+static void configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
u32 cfg2;
/* According to sensor's output format to set cfg2 */
switch (code) {
- /* YUV, including grey */
+ default:
+ /* Grey */
case MEDIA_BUS_FMT_Y8_1X8:
cfg2 = ISI_CFG2_GRAYSCALE;
break;
+ /* YUV */
case MEDIA_BUS_FMT_VYUY8_2X8:
cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
@@ -127,8 +129,6 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
- default:
- return -EINVAL;
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
@@ -139,8 +139,29 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
cfg2 |= ((height - 1) ISI_CFG2_IM_VSIZE_OFFSET)
ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
+}
- return 0;
+static bool is_supported(struct soc_camera_device *icd,
+ const struct soc_camera_format_xlate *xlate)
+{
+ bool ret = true;
+
+ switch (xlate-code) {
+ /* YUV, including grey */
+ case MEDIA_BUS_FMT_Y8_1X8:
+ case MEDIA_BUS_FMT_VYUY8_2X8:
+ case MEDIA_BUS_FMT_UYVY8_2X8:
+ case MEDIA_BUS_FMT_YVYU8_2X8:
+ case MEDIA_BUS_FMT_YUYV8_2X8:
+ break;
+ /* RGB, TODO */
+ default:
+ dev_err(icd-parent, not supported format: %d\n,
+ xlate-code);
+ ret = false;
+ }
+
+ return ret;
}
static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
@@ -391,10 +412,8 @@ static int start_streaming(struct vb2_queue *vq, unsigned
int count)
/* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
- ret = configure_geometry(isi, icd-user_width, icd-user_height,
+ configure_geometry(isi, icd-user_width, icd-user_height,
icd-current_fmt-code);
- if (ret 0)
- return ret;
spin_lock_irq(isi-lock);
/* Clear any pending interrupt */
@@ -515,6 +534,10 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd,
if (mf-code != xlate-code)
return -EINVAL;
+ /* check with atmel-isi support format */
+ if (!is_supported(icd, xlate))
+ return -EINVAL;
+
pix-width = mf-width;
pix-height = mf-height;
pix-field = mf-field;
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/3] media: atmel-isi: setup the ISI_CFG2 register directly
In the function configure_geometry(), we will setup the ISI CFG2
according to the sensor output format.
It make no sense to just read back the CFG2 register and just set part
of it.
So just set up this register directly makes things simpler.
Currently only support YUV format from camera sensor.
Signed-off-by: Josh Wu josh...@atmel.com
Reviewed-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
Changes in v2:
- add Laurent's reviewed-by tag.
drivers/media/platform/soc_camera/atmel-isi.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 274a6f7..0fd6bc9 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -106,24 +106,25 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
static int configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
- u32 cfg2, cr;
+ u32 cfg2;
+ /* According to sensor's output format to set cfg2 */
switch (code) {
/* YUV, including grey */
case MEDIA_BUS_FMT_Y8_1X8:
- cr = ISI_CFG2_GRAYSCALE;
+ cfg2 = ISI_CFG2_GRAYSCALE;
break;
case MEDIA_BUS_FMT_VYUY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_3;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
case MEDIA_BUS_FMT_UYVY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_2;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_2;
break;
case MEDIA_BUS_FMT_YVYU8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_1;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_1;
break;
case MEDIA_BUS_FMT_YUYV8_2X8:
- cr = ISI_CFG2_YCC_SWAP_DEFAULT;
+ cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
default:
@@ -131,17 +132,10 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
-
- cfg2 = isi_readl(isi, ISI_CFG2);
- /* Set YCC swap mode */
- cfg2 = ~ISI_CFG2_YCC_SWAP_MODE_MASK;
- cfg2 |= cr;
/* Set width */
- cfg2 = ~(ISI_CFG2_IM_HSIZE_MASK);
cfg2 |= ((width - 1) ISI_CFG2_IM_HSIZE_OFFSET)
ISI_CFG2_IM_HSIZE_MASK;
/* Set height */
- cfg2 = ~(ISI_CFG2_IM_VSIZE_MASK);
cfg2 |= ((height - 1) ISI_CFG2_IM_VSIZE_OFFSET)
ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 2/3] ARM: at91/dt: add sama5d2 pinmux
Hi, Ludovic On 7/31/2015 11:08 PM, Ludovic Desroches wrote: Signed-off-by: Ludovic Desroches --- arch/arm/boot/dts/sama5d2-pinfunc.h | 760 1 file changed, 760 insertions(+) create mode 100644 arch/arm/boot/dts/sama5d2-pinfunc.h diff --git a/arch/arm/boot/dts/sama5d2-pinfunc.h b/arch/arm/boot/dts/sama5d2-pinfunc.h new file mode 100644 index 000..046d1d5 --- /dev/null +++ b/arch/arm/boot/dts/sama5d2-pinfunc.h [snip...] +#define PIN_PB24__FLEXCOM3_IO3 PINMUX_PIN(PIN_PB24, 5, 3) +#define PIN_PB24__ISI_D10 PINMUX_PIN(PIN_PB24, 6, 3) As sama5d2 use ISC, so It's better to run s/ISI_/ISC_/ in this file, which is consistent with the datasheet. Best Regards, Josh Wu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 2/3] ARM: at91/dt: add sama5d2 pinmux
Hi, Ludovic On 7/31/2015 11:08 PM, Ludovic Desroches wrote: Signed-off-by: Ludovic Desroches ludovic.desroc...@atmel.com --- arch/arm/boot/dts/sama5d2-pinfunc.h | 760 1 file changed, 760 insertions(+) create mode 100644 arch/arm/boot/dts/sama5d2-pinfunc.h diff --git a/arch/arm/boot/dts/sama5d2-pinfunc.h b/arch/arm/boot/dts/sama5d2-pinfunc.h new file mode 100644 index 000..046d1d5 --- /dev/null +++ b/arch/arm/boot/dts/sama5d2-pinfunc.h [snip...] +#define PIN_PB24__FLEXCOM3_IO3 PINMUX_PIN(PIN_PB24, 5, 3) +#define PIN_PB24__ISI_D10 PINMUX_PIN(PIN_PB24, 6, 3) As sama5d2 use ISC, so It's better to run s/ISI_/ISC_/ in this file, which is consistent with the datasheet. Best Regards, Josh Wu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] media: atmel-isi: move configure_geometry() to start_streaming()
HI, Laurent
On 7/31/2015 10:37 PM, Laurent Pinchart wrote:
Hi Josh,
Thank you for the patch.
On Wednesday 17 June 2015 18:39:39 Josh Wu wrote:
As in set_fmt() function we only need to know which format is been set,
we don't need to access the ISI hardware in this moment.
So move the configure_geometry(), which access the ISI hardware, to
start_streaming() will make code more consistent and simpler.
Signed-off-by: Josh Wu
---
drivers/media/platform/soc_camera/atmel-isi.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c index 8bc40ca..b01086d
100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -390,6 +390,11 @@ static int start_streaming(struct vb2_queue *vq,
unsigned int count) /* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
+ ret = configure_geometry(isi, icd->user_width, icd->user_height,
+ icd->current_fmt->code);
I would also make configure_geometry a void function, as the only failure case
really can't occur.
I think this case can be reached if user require a RGB565 format to
capture and sensor also support RGB565 format.
As atmel-isi driver will provide RGB565 support via the pass-through
mode (maybe we need re-consider this part).
So that will cause the configure_geometry() returns an error since it
found the bus format is not Y8 or YUV422.
In my opinion, we should not change configure_geometry()'s return type,
until we add a insanity format check before we call configure_geometry()
in future.
Apart from that,
Reviewed-by: Laurent Pinchart
Thanks for the review.
Best Regards,
Josh Wu
+ if (ret < 0)
+ return ret;
+
spin_lock_irq(>lock);
/* Clear any pending interrupt */
isi_readl(isi, ISI_STATUS);
@@ -477,8 +482,6 @@ static int isi_camera_init_videobuf(struct vb2_queue *q,
static int isi_camera_set_fmt(struct soc_camera_device *icd,
struct v4l2_format *f)
{
- struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
- struct atmel_isi *isi = ici->priv;
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
const struct soc_camera_format_xlate *xlate;
struct v4l2_pix_format *pix = >fmt.pix;
@@ -511,16 +514,6 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd, if (mf->code != xlate->code)
return -EINVAL;
- /* Enable PM and peripheral clock before operate isi registers */
- pm_runtime_get_sync(ici->v4l2_dev.dev);
-
- ret = configure_geometry(isi, pix->width, pix->height, xlate->code);
-
- pm_runtime_put(ici->v4l2_dev.dev);
-
- if (ret < 0)
- return ret;
-
pix->width = mf->width;
pix->height = mf->height;
pix->field = mf->field;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] media: atmel-isi: move configure_geometry() to start_streaming()
HI, Laurent
On 7/31/2015 10:37 PM, Laurent Pinchart wrote:
Hi Josh,
Thank you for the patch.
On Wednesday 17 June 2015 18:39:39 Josh Wu wrote:
As in set_fmt() function we only need to know which format is been set,
we don't need to access the ISI hardware in this moment.
So move the configure_geometry(), which access the ISI hardware, to
start_streaming() will make code more consistent and simpler.
Signed-off-by: Josh Wu josh...@atmel.com
---
drivers/media/platform/soc_camera/atmel-isi.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c index 8bc40ca..b01086d
100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -390,6 +390,11 @@ static int start_streaming(struct vb2_queue *vq,
unsigned int count) /* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
+ ret = configure_geometry(isi, icd-user_width, icd-user_height,
+ icd-current_fmt-code);
I would also make configure_geometry a void function, as the only failure case
really can't occur.
I think this case can be reached if user require a RGB565 format to
capture and sensor also support RGB565 format.
As atmel-isi driver will provide RGB565 support via the pass-through
mode (maybe we need re-consider this part).
So that will cause the configure_geometry() returns an error since it
found the bus format is not Y8 or YUV422.
In my opinion, we should not change configure_geometry()'s return type,
until we add a insanity format check before we call configure_geometry()
in future.
Apart from that,
Reviewed-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Thanks for the review.
Best Regards,
Josh Wu
+ if (ret 0)
+ return ret;
+
spin_lock_irq(isi-lock);
/* Clear any pending interrupt */
isi_readl(isi, ISI_STATUS);
@@ -477,8 +482,6 @@ static int isi_camera_init_videobuf(struct vb2_queue *q,
static int isi_camera_set_fmt(struct soc_camera_device *icd,
struct v4l2_format *f)
{
- struct soc_camera_host *ici = to_soc_camera_host(icd-parent);
- struct atmel_isi *isi = ici-priv;
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
const struct soc_camera_format_xlate *xlate;
struct v4l2_pix_format *pix = f-fmt.pix;
@@ -511,16 +514,6 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd, if (mf-code != xlate-code)
return -EINVAL;
- /* Enable PM and peripheral clock before operate isi registers */
- pm_runtime_get_sync(ici-v4l2_dev.dev);
-
- ret = configure_geometry(isi, pix-width, pix-height, xlate-code);
-
- pm_runtime_put(ici-v4l2_dev.dev);
-
- if (ret 0)
- return ret;
-
pix-width = mf-width;
pix-height = mf-height;
pix-field = mf-field;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] media: atmel-isi: setup the ISI_CFG2 register directly
Hi, list
Ping..., any feedback for this series?
Best Regards,
Josh Wu
On 6/17/2015 6:39 PM, Josh Wu wrote:
In the function configure_geometry(), we will setup the ISI CFG2
according to the sensor output format.
It make no sense to just read back the CFG2 register and just set part
of it.
So just set up this register directly makes things simpler.
Currently only support YUV format from camera sensor.
Signed-off-by: Josh Wu
---
drivers/media/platform/soc_camera/atmel-isi.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 9070172..8bc40ca 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -105,24 +105,25 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
static int configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
- u32 cfg2, cr;
+ u32 cfg2;
+ /* According to sensor's output format to set cfg2 */
switch (code) {
/* YUV, including grey */
case MEDIA_BUS_FMT_Y8_1X8:
- cr = ISI_CFG2_GRAYSCALE;
+ cfg2 = ISI_CFG2_GRAYSCALE;
break;
case MEDIA_BUS_FMT_VYUY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_3;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
case MEDIA_BUS_FMT_UYVY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_2;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_2;
break;
case MEDIA_BUS_FMT_YVYU8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_1;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_1;
break;
case MEDIA_BUS_FMT_YUYV8_2X8:
- cr = ISI_CFG2_YCC_SWAP_DEFAULT;
+ cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
default:
@@ -130,17 +131,10 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
-
- cfg2 = isi_readl(isi, ISI_CFG2);
- /* Set YCC swap mode */
- cfg2 &= ~ISI_CFG2_YCC_SWAP_MODE_MASK;
- cfg2 |= cr;
/* Set width */
- cfg2 &= ~(ISI_CFG2_IM_HSIZE_MASK);
cfg2 |= ((width - 1) << ISI_CFG2_IM_HSIZE_OFFSET) &
ISI_CFG2_IM_HSIZE_MASK;
/* Set height */
- cfg2 &= ~(ISI_CFG2_IM_VSIZE_MASK);
cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
& ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] media: atmel-isi: setup the ISI_CFG2 register directly
Hi, list
Ping..., any feedback for this series?
Best Regards,
Josh Wu
On 6/17/2015 6:39 PM, Josh Wu wrote:
In the function configure_geometry(), we will setup the ISI CFG2
according to the sensor output format.
It make no sense to just read back the CFG2 register and just set part
of it.
So just set up this register directly makes things simpler.
Currently only support YUV format from camera sensor.
Signed-off-by: Josh Wu josh...@atmel.com
---
drivers/media/platform/soc_camera/atmel-isi.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 9070172..8bc40ca 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -105,24 +105,25 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
static int configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
- u32 cfg2, cr;
+ u32 cfg2;
+ /* According to sensor's output format to set cfg2 */
switch (code) {
/* YUV, including grey */
case MEDIA_BUS_FMT_Y8_1X8:
- cr = ISI_CFG2_GRAYSCALE;
+ cfg2 = ISI_CFG2_GRAYSCALE;
break;
case MEDIA_BUS_FMT_VYUY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_3;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
case MEDIA_BUS_FMT_UYVY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_2;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_2;
break;
case MEDIA_BUS_FMT_YVYU8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_1;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_1;
break;
case MEDIA_BUS_FMT_YUYV8_2X8:
- cr = ISI_CFG2_YCC_SWAP_DEFAULT;
+ cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
default:
@@ -130,17 +131,10 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
-
- cfg2 = isi_readl(isi, ISI_CFG2);
- /* Set YCC swap mode */
- cfg2 = ~ISI_CFG2_YCC_SWAP_MODE_MASK;
- cfg2 |= cr;
/* Set width */
- cfg2 = ~(ISI_CFG2_IM_HSIZE_MASK);
cfg2 |= ((width - 1) ISI_CFG2_IM_HSIZE_OFFSET)
ISI_CFG2_IM_HSIZE_MASK;
/* Set height */
- cfg2 = ~(ISI_CFG2_IM_VSIZE_MASK);
cfg2 |= ((height - 1) ISI_CFG2_IM_VSIZE_OFFSET)
ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] ARM: at91: at91_dt_defconfig: enable ISI and ov2640 support
On 7/28/2015 3:37 PM, Nicolas Ferre wrote: Le 16/06/2015 12:08, Josh Wu a écrit : Add Atmel-isi and ov2640 driver in defconfig Signed-off-by: Josh Wu --- Changes in v2: None arch/arm/configs/at91_dt_defconfig | 6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/arm/configs/at91_dt_defconfig b/arch/arm/configs/at91_dt_defconfig index bcef49a..d089ec9 100644 --- a/arch/arm/configs/at91_dt_defconfig +++ b/arch/arm/configs/at91_dt_defconfig @@ -131,6 +131,12 @@ CONFIG_POWER_RESET=y CONFIG_WATCHDOG=y CONFIG_AT91SAM9X_WATCHDOG=y CONFIG_SSB=m +CONFIG_MEDIA_SUPPORT=y +CONFIG_MEDIA_CAMERA_SUPPORT=y +CONFIG_V4L_PLATFORM_DRIVERS=y +CONFIG_SOC_CAMERA=y +CONFIG_VIDEO_ATMEL_ISI=y +CONFIG_SOC_CAMERA_OV2640=y I would select this camera driver as a module. Your thoughts? yes, I agree. Do I need to resent the patch for this? Best Regards, Josh Wu Bye, CONFIG_FB=y CONFIG_FB_ATMEL=y CONFIG_BACKLIGHT_LCD_SUPPORT=y -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] ARM: at91: at91_dt_defconfig: enable ISI and ov2640 support
On 7/28/2015 3:37 PM, Nicolas Ferre wrote: Le 16/06/2015 12:08, Josh Wu a écrit : Add Atmel-isi and ov2640 driver in defconfig Signed-off-by: Josh Wu josh...@atmel.com --- Changes in v2: None arch/arm/configs/at91_dt_defconfig | 6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/arm/configs/at91_dt_defconfig b/arch/arm/configs/at91_dt_defconfig index bcef49a..d089ec9 100644 --- a/arch/arm/configs/at91_dt_defconfig +++ b/arch/arm/configs/at91_dt_defconfig @@ -131,6 +131,12 @@ CONFIG_POWER_RESET=y CONFIG_WATCHDOG=y CONFIG_AT91SAM9X_WATCHDOG=y CONFIG_SSB=m +CONFIG_MEDIA_SUPPORT=y +CONFIG_MEDIA_CAMERA_SUPPORT=y +CONFIG_V4L_PLATFORM_DRIVERS=y +CONFIG_SOC_CAMERA=y +CONFIG_VIDEO_ATMEL_ISI=y +CONFIG_SOC_CAMERA_OV2640=y I would select this camera driver as a module. Your thoughts? yes, I agree. Do I need to resent the patch for this? Best Regards, Josh Wu Bye, CONFIG_FB=y CONFIG_FB_ATMEL=y CONFIG_BACKLIGHT_LCD_SUPPORT=y -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/2] ARM: at91: sama5/dt: update rstc to correct compatible string
They'll use "atmel,sama5d3-rstc" for reset function.
Cc: devicet...@vger.kernel.org
Signed-off-by: Josh Wu
---
Changes in v2: None
arch/arm/boot/dts/sama5d3.dtsi | 2 +-
arch/arm/boot/dts/sama5d4.dtsi | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index 9e2444b..280255b 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -1259,7 +1259,7 @@
};
rstc@fe00 {
- compatible = "atmel,at91sam9g45-rstc";
+ compatible = "atmel,sama5d3-rstc";
reg = <0xfe00 0x10>;
};
diff --git a/arch/arm/boot/dts/sama5d4.dtsi b/arch/arm/boot/dts/sama5d4.dtsi
index 3ee22ee..481196c 100644
--- a/arch/arm/boot/dts/sama5d4.dtsi
+++ b/arch/arm/boot/dts/sama5d4.dtsi
@@ -1277,7 +1277,7 @@
};
rstc@fc068600 {
- compatible = "atmel,at91sam9g45-rstc";
+ compatible = "atmel,sama5d3-rstc";
reg = <0xfc068600 0x10>;
};
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/2] power: reset: at91: add sama5d3 reset function
This patch introduces a new compatible string: "atmel,sama5d3-rstc" and
new reset function for sama5d3 and later chips.
As in sama5d3 or later chips, we don't have to shutdown the DDR
controller before reset. Shutdown the DDR controller before reset is a
workaround to avoid DDR signal driving the bus, but since sama5d3 and
later chips there is no such a conflict.
So in this patch:
1. the sama5d3 reset function only need to write the rstc register
and return.
2. we can remove the code related with sama5d3 DDR controller as
we don't use it at all.
Signed-off-by: Josh Wu
Acked-by: Nicolas Ferre
---
Changes in v2:
- aligned the function parameters to be consist with the coding style
- refined the commit log
- add binding document changes
- use of_device_is_compitable() instead
.../devicetree/bindings/arm/atmel-at91.txt | 2 +-
drivers/power/reset/at91-reset.c | 26 --
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/atmel-at91.txt
b/Documentation/devicetree/bindings/arm/atmel-at91.txt
index 424ac8c..dd998b9 100644
--- a/Documentation/devicetree/bindings/arm/atmel-at91.txt
+++ b/Documentation/devicetree/bindings/arm/atmel-at91.txt
@@ -87,7 +87,7 @@ One interrupt per TC channel in a TC block:
RSTC Reset Controller required properties:
- compatible: Should be "atmel,-rstc".
- can be "at91sam9260" or "at91sam9g45"
+ can be "at91sam9260" or "at91sam9g45" or "sama5d3"
- reg: Should contain registers location and length
Example:
diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index 36dc52f..c378d4e 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -123,6 +123,15 @@ static int at91sam9g45_restart(struct notifier_block
*this, unsigned long mode,
return NOTIFY_DONE;
}
+static int sama5d3_restart(struct notifier_block *this, unsigned long mode,
+ void *cmd)
+{
+ writel(cpu_to_le32(AT91_RSTC_KEY | AT91_RSTC_PERRST |
AT91_RSTC_PROCRST),
+ at91_rstc_base);
+
+ return NOTIFY_DONE;
+}
+
static void __init at91_reset_status(struct platform_device *pdev)
{
u32 reg = readl(at91_rstc_base + AT91_RSTC_SR);
@@ -155,13 +164,13 @@ static void __init at91_reset_status(struct
platform_device *pdev)
static const struct of_device_id at91_ramc_of_match[] = {
{ .compatible = "atmel,at91sam9260-sdramc", },
{ .compatible = "atmel,at91sam9g45-ddramc", },
- { .compatible = "atmel,sama5d3-ddramc", },
{ /* sentinel */ }
};
static const struct of_device_id at91_reset_of_match[] = {
{ .compatible = "atmel,at91sam9260-rstc", .data = at91sam9260_restart },
{ .compatible = "atmel,at91sam9g45-rstc", .data = at91sam9g45_restart },
+ { .compatible = "atmel,sama5d3-rstc", .data = sama5d3_restart },
{ /* sentinel */ }
};
@@ -181,13 +190,16 @@ static int at91_reset_of_probe(struct platform_device
*pdev)
return -ENODEV;
}
- for_each_matching_node(np, at91_ramc_of_match) {
- at91_ramc_base[idx] = of_iomap(np, 0);
- if (!at91_ramc_base[idx]) {
- dev_err(>dev, "Could not map ram controller
address\n");
- return -ENODEV;
+ if (!of_device_is_compatible(pdev->dev.of_node, "atmel,sama5d3-rstc")) {
+ /* we need to shutdown the ddr controller, so get ramc base */
+ for_each_matching_node(np, at91_ramc_of_match) {
+ at91_ramc_base[idx] = of_iomap(np, 0);
+ if (!at91_ramc_base[idx]) {
+ dev_err(>dev, "Could not map ram
controller address\n");
+ return -ENODEV;
+ }
+ idx++;
}
- idx++;
}
match = of_match_node(at91_reset_of_match, pdev->dev.of_node);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
On 7/20/2015 4:44 PM, Josh Wu wrote: On 7/20/2015 4:35 PM, Josh Wu wrote: Hi, Maxime On 7/20/2015 3:52 PM, Maxime Ripard wrote: Hi Josh, On Mon, Jul 13, 2015 at 11:21:44AM +0800, Josh Wu wrote: On 7/11/2015 12:12 AM, Nicolas Ferre wrote: Le 10/07/2015 14:31, Maxime Ripard a écrit : On Fri, Jul 10, 2015 at 02:09:07PM +0200, Alexandre Belloni wrote: Hi, On 10/07/2015 at 15:56:52 +0800, Josh Wu wrote : I would agree with Maxime. Currently all latest chip reset function is compatible with the atmel,sama5d3-rstc. So check compatible string is enough for now. But of cause if we have other incompatible reset in future with new chip, the structure like you said is needed. We managed to avoid using of_machine_is_compatible() in all the at91 drivers. I'd like to keep it that way. It was painful enough to remove all those cpu_is_at91xxx calls. That's your call... Also, using it is trying to match strings and will result in longer boot times. Have you looked at the implementation of of_match_device? If that's really a concern to you, you should actually avoid it. I agree: let's keep it simple and use of_match_device(). Ok. I will keep it as it is now: use the (match->data != sama5d3_restart) for the condition. I'm not just that's been an option in our discussion so far. Nicolas said that he was agreeing with me, but at the same time said the complete opposite of what I was arguing for, so I'm not really sure what's really on his mind, but the two options that were discussed were to remove that test, and either: - Use of_device_is_compatible to prevent the loop execution Thank you for explaining, it is clear to me. I'll take this above option. As the of_device_is_compatible() almost same as of_match_node()/of_match_device(). Except that of_device_is_compatible() is more efficient (in this case It calls __of_device_is_compatible() directly) than of_match_node/of_match_device. Sorry, after checking the code a little, I'd say use the of_match_node instead of of_device_is_compatible() is better. Since After check the of_device_is_compatible() we also need to call of_match_node() again. Okay, Please forget above reply. As Maxime said test the pointer is not good solution here. So I'll sent out v2 which use of_device_is_compatible(). Best Regards, Josh Wu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
On 7/20/2015 4:35 PM, Josh Wu wrote:
Hi, Maxime
On 7/20/2015 3:52 PM, Maxime Ripard wrote:
Hi Josh,
On Mon, Jul 13, 2015 at 11:21:44AM +0800, Josh Wu wrote:
On 7/11/2015 12:12 AM, Nicolas Ferre wrote:
Le 10/07/2015 14:31, Maxime Ripard a écrit :
On Fri, Jul 10, 2015 at 02:09:07PM +0200, Alexandre Belloni wrote:
Hi,
On 10/07/2015 at 15:56:52 +0800, Josh Wu wrote :
I would agree with Maxime. Currently all latest chip reset
function is
compatible with the atmel,sama5d3-rstc.
So check compatible string is enough for now.
But of cause if we have other incompatible reset in future with
new chip,
the structure like you said is needed.
We managed to avoid using of_machine_is_compatible() in all the at91
drivers. I'd like to keep it that way. It was painful enough to
remove
all those cpu_is_at91xxx calls.
That's your call...
Also, using it is trying to match strings and will result in
longer boot
times.
Have you looked at the implementation of of_match_device? If that's
really a concern to you, you should actually avoid it.
I agree: let's keep it simple and use of_match_device().
Ok. I will keep it as it is now: use the (match->data !=
sama5d3_restart)
for the condition.
I'm not just that's been an option in our discussion so far.
Nicolas said that he was agreeing with me, but at the same time said
the complete opposite of what I was arguing for, so I'm not really
sure what's really on his mind, but the two options that were
discussed were to remove that test, and either:
- Use of_device_is_compatible to prevent the loop execution
Thank you for explaining, it is clear to me.
I'll take this above option. As the of_device_is_compatible() almost
same as of_match_node()/of_match_device(). Except that
of_device_is_compatible() is more efficient (in this case It calls
__of_device_is_compatible() directly) than of_match_node/of_match_device.
Sorry, after checking the code a little, I'd say use the of_match_node
instead of of_device_is_compatible() is better. Since After check the
of_device_is_compatible() we also need to call of_match_node() again.
So the simplest way is just get the match data by of_match_node() first,
then check the match->data. like following:
match = of_match_node(at91_reset_of_match, pdev->dev.of_node);
if (match->data != sama5d3_restart) {
/* we need to shutdown the ddr controller, so get ramc base */
for_each_matching_node(np, at91_ramc_of_match) {
at91_ramc_base[idx] = of_iomap(np, 0);
if (!at91_ramc_base[idx]) {
dev_err(>dev, "Could not map ram controller
address\n");
return -ENODEV;
}
idx++;
}
}
at91_restart_nb.notifier_call = match->data;
Best Regards,
Josh Wu
- define a structure with a flag to say whether you need the ram
controller quirk or not, and test that flag.
Maxime
Best Regards,
Josh Wu
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
Hi, Maxime On 7/20/2015 3:52 PM, Maxime Ripard wrote: Hi Josh, On Mon, Jul 13, 2015 at 11:21:44AM +0800, Josh Wu wrote: On 7/11/2015 12:12 AM, Nicolas Ferre wrote: Le 10/07/2015 14:31, Maxime Ripard a écrit : On Fri, Jul 10, 2015 at 02:09:07PM +0200, Alexandre Belloni wrote: Hi, On 10/07/2015 at 15:56:52 +0800, Josh Wu wrote : I would agree with Maxime. Currently all latest chip reset function is compatible with the atmel,sama5d3-rstc. So check compatible string is enough for now. But of cause if we have other incompatible reset in future with new chip, the structure like you said is needed. We managed to avoid using of_machine_is_compatible() in all the at91 drivers. I'd like to keep it that way. It was painful enough to remove all those cpu_is_at91xxx calls. That's your call... Also, using it is trying to match strings and will result in longer boot times. Have you looked at the implementation of of_match_device? If that's really a concern to you, you should actually avoid it. I agree: let's keep it simple and use of_match_device(). Ok. I will keep it as it is now: use the (match->data != sama5d3_restart) for the condition. I'm not just that's been an option in our discussion so far. Nicolas said that he was agreeing with me, but at the same time said the complete opposite of what I was arguing for, so I'm not really sure what's really on his mind, but the two options that were discussed were to remove that test, and either: - Use of_device_is_compatible to prevent the loop execution Thank you for explaining, it is clear to me. I'll take this above option. As the of_device_is_compatible() almost same as of_match_node()/of_match_device(). Except that of_device_is_compatible() is more efficient (in this case It calls __of_device_is_compatible() directly) than of_match_node/of_match_device. - define a structure with a flag to say whether you need the ram controller quirk or not, and test that flag. Maxime Best Regards, Josh Wu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
On 7/20/2015 4:35 PM, Josh Wu wrote:
Hi, Maxime
On 7/20/2015 3:52 PM, Maxime Ripard wrote:
Hi Josh,
On Mon, Jul 13, 2015 at 11:21:44AM +0800, Josh Wu wrote:
On 7/11/2015 12:12 AM, Nicolas Ferre wrote:
Le 10/07/2015 14:31, Maxime Ripard a écrit :
On Fri, Jul 10, 2015 at 02:09:07PM +0200, Alexandre Belloni wrote:
Hi,
On 10/07/2015 at 15:56:52 +0800, Josh Wu wrote :
I would agree with Maxime. Currently all latest chip reset
function is
compatible with the atmel,sama5d3-rstc.
So check compatible string is enough for now.
But of cause if we have other incompatible reset in future with
new chip,
the structure like you said is needed.
We managed to avoid using of_machine_is_compatible() in all the at91
drivers. I'd like to keep it that way. It was painful enough to
remove
all those cpu_is_at91xxx calls.
That's your call...
Also, using it is trying to match strings and will result in
longer boot
times.
Have you looked at the implementation of of_match_device? If that's
really a concern to you, you should actually avoid it.
I agree: let's keep it simple and use of_match_device().
Ok. I will keep it as it is now: use the (match-data !=
sama5d3_restart)
for the condition.
I'm not just that's been an option in our discussion so far.
Nicolas said that he was agreeing with me, but at the same time said
the complete opposite of what I was arguing for, so I'm not really
sure what's really on his mind, but the two options that were
discussed were to remove that test, and either:
- Use of_device_is_compatible to prevent the loop execution
Thank you for explaining, it is clear to me.
I'll take this above option. As the of_device_is_compatible() almost
same as of_match_node()/of_match_device(). Except that
of_device_is_compatible() is more efficient (in this case It calls
__of_device_is_compatible() directly) than of_match_node/of_match_device.
Sorry, after checking the code a little, I'd say use the of_match_node
instead of of_device_is_compatible() is better. Since After check the
of_device_is_compatible() we also need to call of_match_node() again.
So the simplest way is just get the match data by of_match_node() first,
then check the match-data. like following:
match = of_match_node(at91_reset_of_match, pdev-dev.of_node);
if (match-data != sama5d3_restart) {
/* we need to shutdown the ddr controller, so get ramc base */
for_each_matching_node(np, at91_ramc_of_match) {
at91_ramc_base[idx] = of_iomap(np, 0);
if (!at91_ramc_base[idx]) {
dev_err(pdev-dev, Could not map ram controller
address\n);
return -ENODEV;
}
idx++;
}
}
at91_restart_nb.notifier_call = match-data;
Best Regards,
Josh Wu
- define a structure with a flag to say whether you need the ram
controller quirk or not, and test that flag.
Maxime
Best Regards,
Josh Wu
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/2] power: reset: at91: add sama5d3 reset function
This patch introduces a new compatible string: atmel,sama5d3-rstc and
new reset function for sama5d3 and later chips.
As in sama5d3 or later chips, we don't have to shutdown the DDR
controller before reset. Shutdown the DDR controller before reset is a
workaround to avoid DDR signal driving the bus, but since sama5d3 and
later chips there is no such a conflict.
So in this patch:
1. the sama5d3 reset function only need to write the rstc register
and return.
2. we can remove the code related with sama5d3 DDR controller as
we don't use it at all.
Signed-off-by: Josh Wu josh...@atmel.com
Acked-by: Nicolas Ferre nicolas.fe...@atmel.com
---
Changes in v2:
- aligned the function parameters to be consist with the coding style
- refined the commit log
- add binding document changes
- use of_device_is_compitable() instead
.../devicetree/bindings/arm/atmel-at91.txt | 2 +-
drivers/power/reset/at91-reset.c | 26 --
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/atmel-at91.txt
b/Documentation/devicetree/bindings/arm/atmel-at91.txt
index 424ac8c..dd998b9 100644
--- a/Documentation/devicetree/bindings/arm/atmel-at91.txt
+++ b/Documentation/devicetree/bindings/arm/atmel-at91.txt
@@ -87,7 +87,7 @@ One interrupt per TC channel in a TC block:
RSTC Reset Controller required properties:
- compatible: Should be atmel,chip-rstc.
- chip can be at91sam9260 or at91sam9g45
+ chip can be at91sam9260 or at91sam9g45 or sama5d3
- reg: Should contain registers location and length
Example:
diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index 36dc52f..c378d4e 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -123,6 +123,15 @@ static int at91sam9g45_restart(struct notifier_block
*this, unsigned long mode,
return NOTIFY_DONE;
}
+static int sama5d3_restart(struct notifier_block *this, unsigned long mode,
+ void *cmd)
+{
+ writel(cpu_to_le32(AT91_RSTC_KEY | AT91_RSTC_PERRST |
AT91_RSTC_PROCRST),
+ at91_rstc_base);
+
+ return NOTIFY_DONE;
+}
+
static void __init at91_reset_status(struct platform_device *pdev)
{
u32 reg = readl(at91_rstc_base + AT91_RSTC_SR);
@@ -155,13 +164,13 @@ static void __init at91_reset_status(struct
platform_device *pdev)
static const struct of_device_id at91_ramc_of_match[] = {
{ .compatible = atmel,at91sam9260-sdramc, },
{ .compatible = atmel,at91sam9g45-ddramc, },
- { .compatible = atmel,sama5d3-ddramc, },
{ /* sentinel */ }
};
static const struct of_device_id at91_reset_of_match[] = {
{ .compatible = atmel,at91sam9260-rstc, .data = at91sam9260_restart },
{ .compatible = atmel,at91sam9g45-rstc, .data = at91sam9g45_restart },
+ { .compatible = atmel,sama5d3-rstc, .data = sama5d3_restart },
{ /* sentinel */ }
};
@@ -181,13 +190,16 @@ static int at91_reset_of_probe(struct platform_device
*pdev)
return -ENODEV;
}
- for_each_matching_node(np, at91_ramc_of_match) {
- at91_ramc_base[idx] = of_iomap(np, 0);
- if (!at91_ramc_base[idx]) {
- dev_err(pdev-dev, Could not map ram controller
address\n);
- return -ENODEV;
+ if (!of_device_is_compatible(pdev-dev.of_node, atmel,sama5d3-rstc)) {
+ /* we need to shutdown the ddr controller, so get ramc base */
+ for_each_matching_node(np, at91_ramc_of_match) {
+ at91_ramc_base[idx] = of_iomap(np, 0);
+ if (!at91_ramc_base[idx]) {
+ dev_err(pdev-dev, Could not map ram
controller address\n);
+ return -ENODEV;
+ }
+ idx++;
}
- idx++;
}
match = of_match_node(at91_reset_of_match, pdev-dev.of_node);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/2] ARM: at91: sama5/dt: update rstc to correct compatible string
They'll use atmel,sama5d3-rstc for reset function.
Cc: devicet...@vger.kernel.org
Signed-off-by: Josh Wu josh...@atmel.com
---
Changes in v2: None
arch/arm/boot/dts/sama5d3.dtsi | 2 +-
arch/arm/boot/dts/sama5d4.dtsi | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index 9e2444b..280255b 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -1259,7 +1259,7 @@
};
rstc@fe00 {
- compatible = atmel,at91sam9g45-rstc;
+ compatible = atmel,sama5d3-rstc;
reg = 0xfe00 0x10;
};
diff --git a/arch/arm/boot/dts/sama5d4.dtsi b/arch/arm/boot/dts/sama5d4.dtsi
index 3ee22ee..481196c 100644
--- a/arch/arm/boot/dts/sama5d4.dtsi
+++ b/arch/arm/boot/dts/sama5d4.dtsi
@@ -1277,7 +1277,7 @@
};
rstc@fc068600 {
- compatible = atmel,at91sam9g45-rstc;
+ compatible = atmel,sama5d3-rstc;
reg = 0xfc068600 0x10;
};
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
On 7/20/2015 4:44 PM, Josh Wu wrote: On 7/20/2015 4:35 PM, Josh Wu wrote: Hi, Maxime On 7/20/2015 3:52 PM, Maxime Ripard wrote: Hi Josh, On Mon, Jul 13, 2015 at 11:21:44AM +0800, Josh Wu wrote: On 7/11/2015 12:12 AM, Nicolas Ferre wrote: Le 10/07/2015 14:31, Maxime Ripard a écrit : On Fri, Jul 10, 2015 at 02:09:07PM +0200, Alexandre Belloni wrote: Hi, On 10/07/2015 at 15:56:52 +0800, Josh Wu wrote : I would agree with Maxime. Currently all latest chip reset function is compatible with the atmel,sama5d3-rstc. So check compatible string is enough for now. But of cause if we have other incompatible reset in future with new chip, the structure like you said is needed. We managed to avoid using of_machine_is_compatible() in all the at91 drivers. I'd like to keep it that way. It was painful enough to remove all those cpu_is_at91xxx calls. That's your call... Also, using it is trying to match strings and will result in longer boot times. Have you looked at the implementation of of_match_device? If that's really a concern to you, you should actually avoid it. I agree: let's keep it simple and use of_match_device(). Ok. I will keep it as it is now: use the (match-data != sama5d3_restart) for the condition. I'm not just that's been an option in our discussion so far. Nicolas said that he was agreeing with me, but at the same time said the complete opposite of what I was arguing for, so I'm not really sure what's really on his mind, but the two options that were discussed were to remove that test, and either: - Use of_device_is_compatible to prevent the loop execution Thank you for explaining, it is clear to me. I'll take this above option. As the of_device_is_compatible() almost same as of_match_node()/of_match_device(). Except that of_device_is_compatible() is more efficient (in this case It calls __of_device_is_compatible() directly) than of_match_node/of_match_device. Sorry, after checking the code a little, I'd say use the of_match_node instead of of_device_is_compatible() is better. Since After check the of_device_is_compatible() we also need to call of_match_node() again. Okay, Please forget above reply. As Maxime said test the pointer is not good solution here. So I'll sent out v2 which use of_device_is_compatible(). Best Regards, Josh Wu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
Hi, Maxime On 7/20/2015 3:52 PM, Maxime Ripard wrote: Hi Josh, On Mon, Jul 13, 2015 at 11:21:44AM +0800, Josh Wu wrote: On 7/11/2015 12:12 AM, Nicolas Ferre wrote: Le 10/07/2015 14:31, Maxime Ripard a écrit : On Fri, Jul 10, 2015 at 02:09:07PM +0200, Alexandre Belloni wrote: Hi, On 10/07/2015 at 15:56:52 +0800, Josh Wu wrote : I would agree with Maxime. Currently all latest chip reset function is compatible with the atmel,sama5d3-rstc. So check compatible string is enough for now. But of cause if we have other incompatible reset in future with new chip, the structure like you said is needed. We managed to avoid using of_machine_is_compatible() in all the at91 drivers. I'd like to keep it that way. It was painful enough to remove all those cpu_is_at91xxx calls. That's your call... Also, using it is trying to match strings and will result in longer boot times. Have you looked at the implementation of of_match_device? If that's really a concern to you, you should actually avoid it. I agree: let's keep it simple and use of_match_device(). Ok. I will keep it as it is now: use the (match-data != sama5d3_restart) for the condition. I'm not just that's been an option in our discussion so far. Nicolas said that he was agreeing with me, but at the same time said the complete opposite of what I was arguing for, so I'm not really sure what's really on his mind, but the two options that were discussed were to remove that test, and either: - Use of_device_is_compatible to prevent the loop execution Thank you for explaining, it is clear to me. I'll take this above option. As the of_device_is_compatible() almost same as of_match_node()/of_match_device(). Except that of_device_is_compatible() is more efficient (in this case It calls __of_device_is_compatible() directly) than of_match_node/of_match_device. - define a structure with a flag to say whether you need the ram controller quirk or not, and test that flag. Maxime Best Regards, Josh Wu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
On 7/11/2015 12:12 AM, Nicolas Ferre wrote: Le 10/07/2015 14:31, Maxime Ripard a écrit : On Fri, Jul 10, 2015 at 02:09:07PM +0200, Alexandre Belloni wrote: Hi, On 10/07/2015 at 15:56:52 +0800, Josh Wu wrote : I would agree with Maxime. Currently all latest chip reset function is compatible with the atmel,sama5d3-rstc. So check compatible string is enough for now. But of cause if we have other incompatible reset in future with new chip, the structure like you said is needed. We managed to avoid using of_machine_is_compatible() in all the at91 drivers. I'd like to keep it that way. It was painful enough to remove all those cpu_is_at91xxx calls. That's your call... Also, using it is trying to match strings and will result in longer boot times. Have you looked at the implementation of of_match_device? If that's really a concern to you, you should actually avoid it. I agree: let's keep it simple and use of_match_device(). Ok. I will keep it as it is now: use the (match->data != sama5d3_restart) for the condition. About the of_match_device(), I prefer to keep not changing the code and still use of_match_node(). Since of_match_device() is a wrapper for the of_match_node(). And dev->of_node and at91_reset_of_match is valid, so we can just use of_match_node() directly. Is it sound okay for us? Best Regards, Josh Wu Bye, -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
On 7/11/2015 12:12 AM, Nicolas Ferre wrote: Le 10/07/2015 14:31, Maxime Ripard a écrit : On Fri, Jul 10, 2015 at 02:09:07PM +0200, Alexandre Belloni wrote: Hi, On 10/07/2015 at 15:56:52 +0800, Josh Wu wrote : I would agree with Maxime. Currently all latest chip reset function is compatible with the atmel,sama5d3-rstc. So check compatible string is enough for now. But of cause if we have other incompatible reset in future with new chip, the structure like you said is needed. We managed to avoid using of_machine_is_compatible() in all the at91 drivers. I'd like to keep it that way. It was painful enough to remove all those cpu_is_at91xxx calls. That's your call... Also, using it is trying to match strings and will result in longer boot times. Have you looked at the implementation of of_match_device? If that's really a concern to you, you should actually avoid it. I agree: let's keep it simple and use of_match_device(). Ok. I will keep it as it is now: use the (match-data != sama5d3_restart) for the condition. About the of_match_device(), I prefer to keep not changing the code and still use of_match_node(). Since of_match_device() is a wrapper for the of_match_node(). And dev-of_node and at91_reset_of_match is valid, so we can just use of_match_node() directly. Is it sound okay for us? Best Regards, Josh Wu Bye, -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
On 7/10/2015 2:54 PM, Maxime Ripard wrote:
On Fri, Jul 10, 2015 at 11:06:52AM +0800, Josh Wu wrote:
Hi, Maxime
On 7/9/2015 8:03 PM, Maxime Ripard wrote:
Hi,
On Thu, Jul 09, 2015 at 06:15:46PM +0800, Josh Wu wrote:
As since sama5d3, to reset the chip, we don't need to shutdown the ddr
controller.
So add a new compatible string and new restart function for sama5d3 and
later chips. As we don't use sama5d3 ddr controller, so remove it as
well.
Signed-off-by: Josh Wu
Acked-by: Nicolas Ferre
---
drivers/power/reset/at91-reset.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index 36dc52f..8944b63 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -123,6 +123,14 @@ static int at91sam9g45_restart(struct notifier_block
*this, unsigned long mode,
return NOTIFY_DONE;
}
+static int sama5d3_restart(struct notifier_block *this, unsigned long mode,
+ void *cmd)
+{
+ writel(cpu_to_le32(AT91_RSTC_KEY | AT91_RSTC_PERRST |
AT91_RSTC_PROCRST),
+ at91_rstc_base);
+ return NOTIFY_DONE;
+}
+
static void __init at91_reset_status(struct platform_device *pdev)
{
u32 reg = readl(at91_rstc_base + AT91_RSTC_SR);
@@ -155,13 +163,13 @@ static void __init at91_reset_status(struct
platform_device *pdev)
static const struct of_device_id at91_ramc_of_match[] = {
{ .compatible = "atmel,at91sam9260-sdramc", },
{ .compatible = "atmel,at91sam9g45-ddramc", },
- { .compatible = "atmel,sama5d3-ddramc", },
{ /* sentinel */ }
};
static const struct of_device_id at91_reset_of_match[] = {
{ .compatible = "atmel,at91sam9260-rstc", .data = at91sam9260_restart },
{ .compatible = "atmel,at91sam9g45-rstc", .data = at91sam9g45_restart },
+ { .compatible = "atmel,sama5d3-rstc", .data = sama5d3_restart },
{ /* sentinel */ }
};
@@ -181,17 +189,21 @@ static int at91_reset_of_probe(struct platform_device
*pdev)
return -ENODEV;
}
- for_each_matching_node(np, at91_ramc_of_match) {
- at91_ramc_base[idx] = of_iomap(np, 0);
- if (!at91_ramc_base[idx]) {
- dev_err(>dev, "Could not map ram controller
address\n");
- return -ENODEV;
+ match = of_match_node(at91_reset_of_match, pdev->dev.of_node);
+ at91_restart_nb.notifier_call = match->data;
+
+ if (match->data != sama5d3_restart) {
Using of_device_is_compatible seems more appropriate.
Also, why are you changing the order of this loop and the notifier
registration?
I moved this order because I use the match->data to compare whether is
sama5d3_restart. So I need to move this function (of_match_node) up.
Ah right, my bad.
Still, testing against the kernel pointer is not that great.
It would be great to use something explicit instead, like
of_device_is_compatible.
I agree. I will use of_device_is_compatible() in v2. And that can avoid
the order change in the loop as well. Thanks.
Best Regards,
Josh Wu
Maxime
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
Hi, Alexandre
On 7/10/2015 2:03 PM, Alexandre Belloni wrote:
Hi,
On 09/07/2015 at 18:15:46 +0800, Josh Wu wrote :
As since sama5d3, to reset the chip, we don't need to shutdown the ddr
controller.
So add a new compatible string and new restart function for sama5d3 and
later chips. As we don't use sama5d3 ddr controller, so remove it as
well.
Signed-off-by: Josh Wu
Acked-by: Nicolas Ferre
---
drivers/power/reset/at91-reset.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index 36dc52f..8944b63 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -123,6 +123,14 @@ static int at91sam9g45_restart(struct notifier_block
*this, unsigned long mode,
return NOTIFY_DONE;
}
+static int sama5d3_restart(struct notifier_block *this, unsigned long mode,
+ void *cmd)
Please align that line properly.
Ok.
+{
+ writel(cpu_to_le32(AT91_RSTC_KEY | AT91_RSTC_PERRST |
AT91_RSTC_PROCRST),
+ at91_rstc_base);
That one too.
I'll align them in v2.
+ return NOTIFY_DONE;
+}
+
static void __init at91_reset_status(struct platform_device *pdev)
{
u32 reg = readl(at91_rstc_base + AT91_RSTC_SR);
@@ -155,13 +163,13 @@ static void __init at91_reset_status(struct
platform_device *pdev)
static const struct of_device_id at91_ramc_of_match[] = {
{ .compatible = "atmel,at91sam9260-sdramc", },
{ .compatible = "atmel,at91sam9g45-ddramc", },
- { .compatible = "atmel,sama5d3-ddramc", },
{ /* sentinel */ }
};
static const struct of_device_id at91_reset_of_match[] = {
{ .compatible = "atmel,at91sam9260-rstc", .data = at91sam9260_restart },
{ .compatible = "atmel,at91sam9g45-rstc", .data = at91sam9g45_restart },
+ { .compatible = "atmel,sama5d3-rstc", .data = sama5d3_restart },
{ /* sentinel */ }
};
@@ -181,17 +189,21 @@ static int at91_reset_of_probe(struct platform_device *pdev)
return -ENODEV;
}
- for_each_matching_node(np, at91_ramc_of_match) {
- at91_ramc_base[idx] = of_iomap(np, 0);
- if (!at91_ramc_base[idx]) {
- dev_err(>dev, "Could not map ram controller
address\n");
- return -ENODEV;
+ match = of_match_node(at91_reset_of_match, pdev->dev.of_node);
+ at91_restart_nb.notifier_call = match->data;
+
+ if (match->data != sama5d3_restart) {
This doesn't scale well. I would create a structure with a pointer to
the restart function and a boolean or a bitfield to store whether the
workaround is needed. Use that structure in your match data. Then, you
won't need to reorder anything.
I would agree with Maxime. Currently all latest chip reset function is
compatible with the atmel,sama5d3-rstc.
So check compatible string is enough for now.
But of cause if we have other incompatible reset in future with new
chip, the structure like you said is needed.
Thanks.
Best Regards,
Josh Wu
+ /* we need to shutdown the ddr controller, so get ramc base */
+ for_each_matching_node(np, at91_ramc_of_match) {
+ at91_ramc_base[idx] = of_iomap(np, 0);
+ if (!at91_ramc_base[idx]) {
+ dev_err(>dev, "Could not map ram controller
address\n");
+ return -ENODEV;
+ }
+ idx++;
}
- idx++;
}
- match = of_match_node(at91_reset_of_match, pdev->dev.of_node);
- at91_restart_nb.notifier_call = match->data;
return register_restart_handler(_restart_nb);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
Hi, Alexandre
On 7/10/2015 2:03 PM, Alexandre Belloni wrote:
Hi,
On 09/07/2015 at 18:15:46 +0800, Josh Wu wrote :
As since sama5d3, to reset the chip, we don't need to shutdown the ddr
controller.
So add a new compatible string and new restart function for sama5d3 and
later chips. As we don't use sama5d3 ddr controller, so remove it as
well.
Signed-off-by: Josh Wu josh...@atmel.com
Acked-by: Nicolas Ferre nicolas.fe...@atmel.com
---
drivers/power/reset/at91-reset.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index 36dc52f..8944b63 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -123,6 +123,14 @@ static int at91sam9g45_restart(struct notifier_block
*this, unsigned long mode,
return NOTIFY_DONE;
}
+static int sama5d3_restart(struct notifier_block *this, unsigned long mode,
+ void *cmd)
Please align that line properly.
Ok.
+{
+ writel(cpu_to_le32(AT91_RSTC_KEY | AT91_RSTC_PERRST |
AT91_RSTC_PROCRST),
+ at91_rstc_base);
That one too.
I'll align them in v2.
+ return NOTIFY_DONE;
+}
+
static void __init at91_reset_status(struct platform_device *pdev)
{
u32 reg = readl(at91_rstc_base + AT91_RSTC_SR);
@@ -155,13 +163,13 @@ static void __init at91_reset_status(struct
platform_device *pdev)
static const struct of_device_id at91_ramc_of_match[] = {
{ .compatible = atmel,at91sam9260-sdramc, },
{ .compatible = atmel,at91sam9g45-ddramc, },
- { .compatible = atmel,sama5d3-ddramc, },
{ /* sentinel */ }
};
static const struct of_device_id at91_reset_of_match[] = {
{ .compatible = atmel,at91sam9260-rstc, .data = at91sam9260_restart },
{ .compatible = atmel,at91sam9g45-rstc, .data = at91sam9g45_restart },
+ { .compatible = atmel,sama5d3-rstc, .data = sama5d3_restart },
{ /* sentinel */ }
};
@@ -181,17 +189,21 @@ static int at91_reset_of_probe(struct platform_device *pdev)
return -ENODEV;
}
- for_each_matching_node(np, at91_ramc_of_match) {
- at91_ramc_base[idx] = of_iomap(np, 0);
- if (!at91_ramc_base[idx]) {
- dev_err(pdev-dev, Could not map ram controller
address\n);
- return -ENODEV;
+ match = of_match_node(at91_reset_of_match, pdev-dev.of_node);
+ at91_restart_nb.notifier_call = match-data;
+
+ if (match-data != sama5d3_restart) {
This doesn't scale well. I would create a structure with a pointer to
the restart function and a boolean or a bitfield to store whether the
workaround is needed. Use that structure in your match data. Then, you
won't need to reorder anything.
I would agree with Maxime. Currently all latest chip reset function is
compatible with the atmel,sama5d3-rstc.
So check compatible string is enough for now.
But of cause if we have other incompatible reset in future with new
chip, the structure like you said is needed.
Thanks.
Best Regards,
Josh Wu
+ /* we need to shutdown the ddr controller, so get ramc base */
+ for_each_matching_node(np, at91_ramc_of_match) {
+ at91_ramc_base[idx] = of_iomap(np, 0);
+ if (!at91_ramc_base[idx]) {
+ dev_err(pdev-dev, Could not map ram controller
address\n);
+ return -ENODEV;
+ }
+ idx++;
}
- idx++;
}
- match = of_match_node(at91_reset_of_match, pdev-dev.of_node);
- at91_restart_nb.notifier_call = match-data;
return register_restart_handler(at91_restart_nb);
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
On 7/10/2015 2:54 PM, Maxime Ripard wrote:
On Fri, Jul 10, 2015 at 11:06:52AM +0800, Josh Wu wrote:
Hi, Maxime
On 7/9/2015 8:03 PM, Maxime Ripard wrote:
Hi,
On Thu, Jul 09, 2015 at 06:15:46PM +0800, Josh Wu wrote:
As since sama5d3, to reset the chip, we don't need to shutdown the ddr
controller.
So add a new compatible string and new restart function for sama5d3 and
later chips. As we don't use sama5d3 ddr controller, so remove it as
well.
Signed-off-by: Josh Wu josh...@atmel.com
Acked-by: Nicolas Ferre nicolas.fe...@atmel.com
---
drivers/power/reset/at91-reset.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index 36dc52f..8944b63 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -123,6 +123,14 @@ static int at91sam9g45_restart(struct notifier_block
*this, unsigned long mode,
return NOTIFY_DONE;
}
+static int sama5d3_restart(struct notifier_block *this, unsigned long mode,
+ void *cmd)
+{
+ writel(cpu_to_le32(AT91_RSTC_KEY | AT91_RSTC_PERRST |
AT91_RSTC_PROCRST),
+ at91_rstc_base);
+ return NOTIFY_DONE;
+}
+
static void __init at91_reset_status(struct platform_device *pdev)
{
u32 reg = readl(at91_rstc_base + AT91_RSTC_SR);
@@ -155,13 +163,13 @@ static void __init at91_reset_status(struct
platform_device *pdev)
static const struct of_device_id at91_ramc_of_match[] = {
{ .compatible = atmel,at91sam9260-sdramc, },
{ .compatible = atmel,at91sam9g45-ddramc, },
- { .compatible = atmel,sama5d3-ddramc, },
{ /* sentinel */ }
};
static const struct of_device_id at91_reset_of_match[] = {
{ .compatible = atmel,at91sam9260-rstc, .data = at91sam9260_restart },
{ .compatible = atmel,at91sam9g45-rstc, .data = at91sam9g45_restart },
+ { .compatible = atmel,sama5d3-rstc, .data = sama5d3_restart },
{ /* sentinel */ }
};
@@ -181,17 +189,21 @@ static int at91_reset_of_probe(struct platform_device
*pdev)
return -ENODEV;
}
- for_each_matching_node(np, at91_ramc_of_match) {
- at91_ramc_base[idx] = of_iomap(np, 0);
- if (!at91_ramc_base[idx]) {
- dev_err(pdev-dev, Could not map ram controller
address\n);
- return -ENODEV;
+ match = of_match_node(at91_reset_of_match, pdev-dev.of_node);
+ at91_restart_nb.notifier_call = match-data;
+
+ if (match-data != sama5d3_restart) {
Using of_device_is_compatible seems more appropriate.
Also, why are you changing the order of this loop and the notifier
registration?
I moved this order because I use the match-data to compare whether is
sama5d3_restart. So I need to move this function (of_match_node) up.
Ah right, my bad.
Still, testing against the kernel pointer is not that great.
It would be great to use something explicit instead, like
of_device_is_compatible.
I agree. I will use of_device_is_compatible() in v2. And that can avoid
the order change in the loop as well. Thanks.
Best Regards,
Josh Wu
Maxime
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
Hi, Guenter On 7/10/2015 11:14 AM, Guenter Roeck wrote: On Fri, Jul 10, 2015 at 09:59:53AM +0800, Josh Wu wrote: Hi, Guenter On 7/10/2015 1:37 AM, Guenter Roeck wrote: On Thu, Jul 09, 2015 at 06:15:46PM +0800, Josh Wu wrote: As since sama5d3, to reset the chip, we don't need to shutdown the ddr controller. So add a new compatible string and new restart function for sama5d3 and later chips. As we don't use sama5d3 ddr controller, so remove it as well. That sounds like it should be two separate patches, or am I missing something ? I think using one patch makes more sense. Maybe the commit log is not clear enough. How about put it this way: This patch introduces a new compatible string: "atmel,sama5d3-rstc" for the reset driver of sama5d3 and later chips. As in sama5d3 or later chips, we don't have to shutdown the DDR controller before reset. Shutdown the DDR controller before reset is a workaround to avoid DDR signal driving the bus, but since sama5d3 and later chips there is no such a conflict. That means: 1. the sama5d3 reset function only need to write the rstc register and return. 2. for sama5d3, we can remove the code related with DDR controller as we don't use it at all. Sorry, I don't get it. Doesn't that mean there are two distinct logical changes, which would ask for two separate patches ? The rewritten reset function for sama5d3 has no need to access the ddr controller, so this patch rewrite the reset function and remove ddr access for sama5d3. Those two changes are related and so make it as one patch is reasonable. Best Regards, Josh Wu Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
Hi, Maxime
On 7/9/2015 8:03 PM, Maxime Ripard wrote:
Hi,
On Thu, Jul 09, 2015 at 06:15:46PM +0800, Josh Wu wrote:
As since sama5d3, to reset the chip, we don't need to shutdown the ddr
controller.
So add a new compatible string and new restart function for sama5d3 and
later chips. As we don't use sama5d3 ddr controller, so remove it as
well.
Signed-off-by: Josh Wu
Acked-by: Nicolas Ferre
---
drivers/power/reset/at91-reset.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index 36dc52f..8944b63 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -123,6 +123,14 @@ static int at91sam9g45_restart(struct notifier_block
*this, unsigned long mode,
return NOTIFY_DONE;
}
+static int sama5d3_restart(struct notifier_block *this, unsigned long mode,
+ void *cmd)
+{
+ writel(cpu_to_le32(AT91_RSTC_KEY | AT91_RSTC_PERRST |
AT91_RSTC_PROCRST),
+ at91_rstc_base);
+ return NOTIFY_DONE;
+}
+
static void __init at91_reset_status(struct platform_device *pdev)
{
u32 reg = readl(at91_rstc_base + AT91_RSTC_SR);
@@ -155,13 +163,13 @@ static void __init at91_reset_status(struct
platform_device *pdev)
static const struct of_device_id at91_ramc_of_match[] = {
{ .compatible = "atmel,at91sam9260-sdramc", },
{ .compatible = "atmel,at91sam9g45-ddramc", },
- { .compatible = "atmel,sama5d3-ddramc", },
{ /* sentinel */ }
};
static const struct of_device_id at91_reset_of_match[] = {
{ .compatible = "atmel,at91sam9260-rstc", .data = at91sam9260_restart },
{ .compatible = "atmel,at91sam9g45-rstc", .data = at91sam9g45_restart },
+ { .compatible = "atmel,sama5d3-rstc", .data = sama5d3_restart },
{ /* sentinel */ }
};
@@ -181,17 +189,21 @@ static int at91_reset_of_probe(struct platform_device *pdev)
return -ENODEV;
}
- for_each_matching_node(np, at91_ramc_of_match) {
- at91_ramc_base[idx] = of_iomap(np, 0);
- if (!at91_ramc_base[idx]) {
- dev_err(>dev, "Could not map ram controller
address\n");
- return -ENODEV;
+ match = of_match_node(at91_reset_of_match, pdev->dev.of_node);
+ at91_restart_nb.notifier_call = match->data;
+
+ if (match->data != sama5d3_restart) {
Using of_device_is_compatible seems more appropriate.
Also, why are you changing the order of this loop and the notifier
registration?
I moved this order because I use the match->data to compare whether is
sama5d3_restart. So I need to move this function (of_match_node) up.
Best Regards,
Josh Wu
Maxime
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
Hi, Guenter
On 7/10/2015 1:37 AM, Guenter Roeck wrote:
On Thu, Jul 09, 2015 at 06:15:46PM +0800, Josh Wu wrote:
As since sama5d3, to reset the chip, we don't need to shutdown the ddr
controller.
So add a new compatible string and new restart function for sama5d3 and
later chips. As we don't use sama5d3 ddr controller, so remove it as
well.
That sounds like it should be two separate patches, or am I missing something ?
I think using one patch makes more sense. Maybe the commit log is not
clear enough. How about put it this way:
This patch introduces a new compatible string: "atmel,sama5d3-rstc" for
the reset driver of sama5d3 and later chips.
As in sama5d3 or later chips, we don't have to shutdown the DDR
controller before reset. Shutdown the DDR controller before reset is a
workaround to avoid DDR signal driving the bus, but since sama5d3 and
later chips there is no such a conflict.
That means:
1. the sama5d3 reset function only need to write the rstc register
and return.
2. for sama5d3, we can remove the code related with DDR controller as
we don't use it at all.
Best Regards,
Josh Wu
Guenter
Signed-off-by: Josh Wu
Acked-by: Nicolas Ferre
---
drivers/power/reset/at91-reset.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index 36dc52f..8944b63 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -123,6 +123,14 @@ static int at91sam9g45_restart(struct notifier_block
*this, unsigned long mode,
return NOTIFY_DONE;
}
+static int sama5d3_restart(struct notifier_block *this, unsigned long mode,
+ void *cmd)
+{
+ writel(cpu_to_le32(AT91_RSTC_KEY | AT91_RSTC_PERRST |
AT91_RSTC_PROCRST),
+ at91_rstc_base);
+ return NOTIFY_DONE;
+}
+
static void __init at91_reset_status(struct platform_device *pdev)
{
u32 reg = readl(at91_rstc_base + AT91_RSTC_SR);
@@ -155,13 +163,13 @@ static void __init at91_reset_status(struct
platform_device *pdev)
static const struct of_device_id at91_ramc_of_match[] = {
{ .compatible = "atmel,at91sam9260-sdramc", },
{ .compatible = "atmel,at91sam9g45-ddramc", },
- { .compatible = "atmel,sama5d3-ddramc", },
{ /* sentinel */ }
};
static const struct of_device_id at91_reset_of_match[] = {
{ .compatible = "atmel,at91sam9260-rstc", .data = at91sam9260_restart },
{ .compatible = "atmel,at91sam9g45-rstc", .data = at91sam9g45_restart },
+ { .compatible = "atmel,sama5d3-rstc", .data = sama5d3_restart },
{ /* sentinel */ }
};
@@ -181,17 +189,21 @@ static int at91_reset_of_probe(struct platform_device *pdev)
return -ENODEV;
}
- for_each_matching_node(np, at91_ramc_of_match) {
- at91_ramc_base[idx] = of_iomap(np, 0);
- if (!at91_ramc_base[idx]) {
- dev_err(>dev, "Could not map ram controller
address\n");
- return -ENODEV;
+ match = of_match_node(at91_reset_of_match, pdev->dev.of_node);
+ at91_restart_nb.notifier_call = match->data;
+
+ if (match->data != sama5d3_restart) {
+ /* we need to shutdown the ddr controller, so get ramc base */
+ for_each_matching_node(np, at91_ramc_of_match) {
+ at91_ramc_base[idx] = of_iomap(np, 0);
+ if (!at91_ramc_base[idx]) {
+ dev_err(>dev, "Could not map ram controller
address\n");
+ return -ENODEV;
+ }
+ idx++;
}
- idx++;
}
- match = of_match_node(at91_reset_of_match, pdev->dev.of_node);
- at91_restart_nb.notifier_call = match->data;
return register_restart_handler(_restart_nb);
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] power: reset: at91: add sama5d3 reset function
As since sama5d3, to reset the chip, we don't need to shutdown the ddr
controller.
So add a new compatible string and new restart function for sama5d3 and
later chips. As we don't use sama5d3 ddr controller, so remove it as
well.
Signed-off-by: Josh Wu
Acked-by: Nicolas Ferre
---
drivers/power/reset/at91-reset.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index 36dc52f..8944b63 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -123,6 +123,14 @@ static int at91sam9g45_restart(struct notifier_block
*this, unsigned long mode,
return NOTIFY_DONE;
}
+static int sama5d3_restart(struct notifier_block *this, unsigned long mode,
+ void *cmd)
+{
+ writel(cpu_to_le32(AT91_RSTC_KEY | AT91_RSTC_PERRST |
AT91_RSTC_PROCRST),
+ at91_rstc_base);
+ return NOTIFY_DONE;
+}
+
static void __init at91_reset_status(struct platform_device *pdev)
{
u32 reg = readl(at91_rstc_base + AT91_RSTC_SR);
@@ -155,13 +163,13 @@ static void __init at91_reset_status(struct
platform_device *pdev)
static const struct of_device_id at91_ramc_of_match[] = {
{ .compatible = "atmel,at91sam9260-sdramc", },
{ .compatible = "atmel,at91sam9g45-ddramc", },
- { .compatible = "atmel,sama5d3-ddramc", },
{ /* sentinel */ }
};
static const struct of_device_id at91_reset_of_match[] = {
{ .compatible = "atmel,at91sam9260-rstc", .data = at91sam9260_restart },
{ .compatible = "atmel,at91sam9g45-rstc", .data = at91sam9g45_restart },
+ { .compatible = "atmel,sama5d3-rstc", .data = sama5d3_restart },
{ /* sentinel */ }
};
@@ -181,17 +189,21 @@ static int at91_reset_of_probe(struct platform_device
*pdev)
return -ENODEV;
}
- for_each_matching_node(np, at91_ramc_of_match) {
- at91_ramc_base[idx] = of_iomap(np, 0);
- if (!at91_ramc_base[idx]) {
- dev_err(>dev, "Could not map ram controller
address\n");
- return -ENODEV;
+ match = of_match_node(at91_reset_of_match, pdev->dev.of_node);
+ at91_restart_nb.notifier_call = match->data;
+
+ if (match->data != sama5d3_restart) {
+ /* we need to shutdown the ddr controller, so get ramc base */
+ for_each_matching_node(np, at91_ramc_of_match) {
+ at91_ramc_base[idx] = of_iomap(np, 0);
+ if (!at91_ramc_base[idx]) {
+ dev_err(>dev, "Could not map ram
controller address\n");
+ return -ENODEV;
+ }
+ idx++;
}
- idx++;
}
- match = of_match_node(at91_reset_of_match, pdev->dev.of_node);
- at91_restart_nb.notifier_call = match->data;
return register_restart_handler(_restart_nb);
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] ARM: at91: sama5/dt: update rstc to correct compatible string
They'll use "atmel,sama5d3-rstc" for reset function.
Cc: devicet...@vger.kernel.org
Signed-off-by: Josh Wu
---
arch/arm/boot/dts/sama5d3.dtsi | 2 +-
arch/arm/boot/dts/sama5d4.dtsi | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index 9e2444b..280255b 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -1259,7 +1259,7 @@
};
rstc@fe00 {
- compatible = "atmel,at91sam9g45-rstc";
+ compatible = "atmel,sama5d3-rstc";
reg = <0xfe00 0x10>;
};
diff --git a/arch/arm/boot/dts/sama5d4.dtsi b/arch/arm/boot/dts/sama5d4.dtsi
index 3ee22ee..481196c 100644
--- a/arch/arm/boot/dts/sama5d4.dtsi
+++ b/arch/arm/boot/dts/sama5d4.dtsi
@@ -1277,7 +1277,7 @@
};
rstc@fc068600 {
- compatible = "atmel,at91sam9g45-rstc";
+ compatible = "atmel,sama5d3-rstc";
reg = <0xfc068600 0x10>;
};
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
Hi, Guenter
On 7/10/2015 1:37 AM, Guenter Roeck wrote:
On Thu, Jul 09, 2015 at 06:15:46PM +0800, Josh Wu wrote:
As since sama5d3, to reset the chip, we don't need to shutdown the ddr
controller.
So add a new compatible string and new restart function for sama5d3 and
later chips. As we don't use sama5d3 ddr controller, so remove it as
well.
That sounds like it should be two separate patches, or am I missing something ?
I think using one patch makes more sense. Maybe the commit log is not
clear enough. How about put it this way:
This patch introduces a new compatible string: atmel,sama5d3-rstc for
the reset driver of sama5d3 and later chips.
As in sama5d3 or later chips, we don't have to shutdown the DDR
controller before reset. Shutdown the DDR controller before reset is a
workaround to avoid DDR signal driving the bus, but since sama5d3 and
later chips there is no such a conflict.
That means:
1. the sama5d3 reset function only need to write the rstc register
and return.
2. for sama5d3, we can remove the code related with DDR controller as
we don't use it at all.
Best Regards,
Josh Wu
Guenter
Signed-off-by: Josh Wu josh...@atmel.com
Acked-by: Nicolas Ferre nicolas.fe...@atmel.com
---
drivers/power/reset/at91-reset.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index 36dc52f..8944b63 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -123,6 +123,14 @@ static int at91sam9g45_restart(struct notifier_block
*this, unsigned long mode,
return NOTIFY_DONE;
}
+static int sama5d3_restart(struct notifier_block *this, unsigned long mode,
+ void *cmd)
+{
+ writel(cpu_to_le32(AT91_RSTC_KEY | AT91_RSTC_PERRST |
AT91_RSTC_PROCRST),
+ at91_rstc_base);
+ return NOTIFY_DONE;
+}
+
static void __init at91_reset_status(struct platform_device *pdev)
{
u32 reg = readl(at91_rstc_base + AT91_RSTC_SR);
@@ -155,13 +163,13 @@ static void __init at91_reset_status(struct
platform_device *pdev)
static const struct of_device_id at91_ramc_of_match[] = {
{ .compatible = atmel,at91sam9260-sdramc, },
{ .compatible = atmel,at91sam9g45-ddramc, },
- { .compatible = atmel,sama5d3-ddramc, },
{ /* sentinel */ }
};
static const struct of_device_id at91_reset_of_match[] = {
{ .compatible = atmel,at91sam9260-rstc, .data = at91sam9260_restart },
{ .compatible = atmel,at91sam9g45-rstc, .data = at91sam9g45_restart },
+ { .compatible = atmel,sama5d3-rstc, .data = sama5d3_restart },
{ /* sentinel */ }
};
@@ -181,17 +189,21 @@ static int at91_reset_of_probe(struct platform_device *pdev)
return -ENODEV;
}
- for_each_matching_node(np, at91_ramc_of_match) {
- at91_ramc_base[idx] = of_iomap(np, 0);
- if (!at91_ramc_base[idx]) {
- dev_err(pdev-dev, Could not map ram controller
address\n);
- return -ENODEV;
+ match = of_match_node(at91_reset_of_match, pdev-dev.of_node);
+ at91_restart_nb.notifier_call = match-data;
+
+ if (match-data != sama5d3_restart) {
+ /* we need to shutdown the ddr controller, so get ramc base */
+ for_each_matching_node(np, at91_ramc_of_match) {
+ at91_ramc_base[idx] = of_iomap(np, 0);
+ if (!at91_ramc_base[idx]) {
+ dev_err(pdev-dev, Could not map ram controller
address\n);
+ return -ENODEV;
+ }
+ idx++;
}
- idx++;
}
- match = of_match_node(at91_reset_of_match, pdev-dev.of_node);
- at91_restart_nb.notifier_call = match-data;
return register_restart_handler(at91_restart_nb);
}
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
Hi, Maxime
On 7/9/2015 8:03 PM, Maxime Ripard wrote:
Hi,
On Thu, Jul 09, 2015 at 06:15:46PM +0800, Josh Wu wrote:
As since sama5d3, to reset the chip, we don't need to shutdown the ddr
controller.
So add a new compatible string and new restart function for sama5d3 and
later chips. As we don't use sama5d3 ddr controller, so remove it as
well.
Signed-off-by: Josh Wu josh...@atmel.com
Acked-by: Nicolas Ferre nicolas.fe...@atmel.com
---
drivers/power/reset/at91-reset.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index 36dc52f..8944b63 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -123,6 +123,14 @@ static int at91sam9g45_restart(struct notifier_block
*this, unsigned long mode,
return NOTIFY_DONE;
}
+static int sama5d3_restart(struct notifier_block *this, unsigned long mode,
+ void *cmd)
+{
+ writel(cpu_to_le32(AT91_RSTC_KEY | AT91_RSTC_PERRST |
AT91_RSTC_PROCRST),
+ at91_rstc_base);
+ return NOTIFY_DONE;
+}
+
static void __init at91_reset_status(struct platform_device *pdev)
{
u32 reg = readl(at91_rstc_base + AT91_RSTC_SR);
@@ -155,13 +163,13 @@ static void __init at91_reset_status(struct
platform_device *pdev)
static const struct of_device_id at91_ramc_of_match[] = {
{ .compatible = atmel,at91sam9260-sdramc, },
{ .compatible = atmel,at91sam9g45-ddramc, },
- { .compatible = atmel,sama5d3-ddramc, },
{ /* sentinel */ }
};
static const struct of_device_id at91_reset_of_match[] = {
{ .compatible = atmel,at91sam9260-rstc, .data = at91sam9260_restart },
{ .compatible = atmel,at91sam9g45-rstc, .data = at91sam9g45_restart },
+ { .compatible = atmel,sama5d3-rstc, .data = sama5d3_restart },
{ /* sentinel */ }
};
@@ -181,17 +189,21 @@ static int at91_reset_of_probe(struct platform_device *pdev)
return -ENODEV;
}
- for_each_matching_node(np, at91_ramc_of_match) {
- at91_ramc_base[idx] = of_iomap(np, 0);
- if (!at91_ramc_base[idx]) {
- dev_err(pdev-dev, Could not map ram controller
address\n);
- return -ENODEV;
+ match = of_match_node(at91_reset_of_match, pdev-dev.of_node);
+ at91_restart_nb.notifier_call = match-data;
+
+ if (match-data != sama5d3_restart) {
Using of_device_is_compatible seems more appropriate.
Also, why are you changing the order of this loop and the notifier
registration?
I moved this order because I use the match-data to compare whether is
sama5d3_restart. So I need to move this function (of_match_node) up.
Best Regards,
Josh Wu
Maxime
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] power: reset: at91: add sama5d3 reset function
Hi, Guenter On 7/10/2015 11:14 AM, Guenter Roeck wrote: On Fri, Jul 10, 2015 at 09:59:53AM +0800, Josh Wu wrote: Hi, Guenter On 7/10/2015 1:37 AM, Guenter Roeck wrote: On Thu, Jul 09, 2015 at 06:15:46PM +0800, Josh Wu wrote: As since sama5d3, to reset the chip, we don't need to shutdown the ddr controller. So add a new compatible string and new restart function for sama5d3 and later chips. As we don't use sama5d3 ddr controller, so remove it as well. That sounds like it should be two separate patches, or am I missing something ? I think using one patch makes more sense. Maybe the commit log is not clear enough. How about put it this way: This patch introduces a new compatible string: atmel,sama5d3-rstc for the reset driver of sama5d3 and later chips. As in sama5d3 or later chips, we don't have to shutdown the DDR controller before reset. Shutdown the DDR controller before reset is a workaround to avoid DDR signal driving the bus, but since sama5d3 and later chips there is no such a conflict. That means: 1. the sama5d3 reset function only need to write the rstc register and return. 2. for sama5d3, we can remove the code related with DDR controller as we don't use it at all. Sorry, I don't get it. Doesn't that mean there are two distinct logical changes, which would ask for two separate patches ? The rewritten reset function for sama5d3 has no need to access the ddr controller, so this patch rewrite the reset function and remove ddr access for sama5d3. Those two changes are related and so make it as one patch is reasonable. Best Regards, Josh Wu Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] ARM: at91: sama5/dt: update rstc to correct compatible string
They'll use atmel,sama5d3-rstc for reset function.
Cc: devicet...@vger.kernel.org
Signed-off-by: Josh Wu josh...@atmel.com
---
arch/arm/boot/dts/sama5d3.dtsi | 2 +-
arch/arm/boot/dts/sama5d4.dtsi | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index 9e2444b..280255b 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -1259,7 +1259,7 @@
};
rstc@fe00 {
- compatible = atmel,at91sam9g45-rstc;
+ compatible = atmel,sama5d3-rstc;
reg = 0xfe00 0x10;
};
diff --git a/arch/arm/boot/dts/sama5d4.dtsi b/arch/arm/boot/dts/sama5d4.dtsi
index 3ee22ee..481196c 100644
--- a/arch/arm/boot/dts/sama5d4.dtsi
+++ b/arch/arm/boot/dts/sama5d4.dtsi
@@ -1277,7 +1277,7 @@
};
rstc@fc068600 {
- compatible = atmel,at91sam9g45-rstc;
+ compatible = atmel,sama5d3-rstc;
reg = 0xfc068600 0x10;
};
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] power: reset: at91: add sama5d3 reset function
As since sama5d3, to reset the chip, we don't need to shutdown the ddr
controller.
So add a new compatible string and new restart function for sama5d3 and
later chips. As we don't use sama5d3 ddr controller, so remove it as
well.
Signed-off-by: Josh Wu josh...@atmel.com
Acked-by: Nicolas Ferre nicolas.fe...@atmel.com
---
drivers/power/reset/at91-reset.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index 36dc52f..8944b63 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -123,6 +123,14 @@ static int at91sam9g45_restart(struct notifier_block
*this, unsigned long mode,
return NOTIFY_DONE;
}
+static int sama5d3_restart(struct notifier_block *this, unsigned long mode,
+ void *cmd)
+{
+ writel(cpu_to_le32(AT91_RSTC_KEY | AT91_RSTC_PERRST |
AT91_RSTC_PROCRST),
+ at91_rstc_base);
+ return NOTIFY_DONE;
+}
+
static void __init at91_reset_status(struct platform_device *pdev)
{
u32 reg = readl(at91_rstc_base + AT91_RSTC_SR);
@@ -155,13 +163,13 @@ static void __init at91_reset_status(struct
platform_device *pdev)
static const struct of_device_id at91_ramc_of_match[] = {
{ .compatible = atmel,at91sam9260-sdramc, },
{ .compatible = atmel,at91sam9g45-ddramc, },
- { .compatible = atmel,sama5d3-ddramc, },
{ /* sentinel */ }
};
static const struct of_device_id at91_reset_of_match[] = {
{ .compatible = atmel,at91sam9260-rstc, .data = at91sam9260_restart },
{ .compatible = atmel,at91sam9g45-rstc, .data = at91sam9g45_restart },
+ { .compatible = atmel,sama5d3-rstc, .data = sama5d3_restart },
{ /* sentinel */ }
};
@@ -181,17 +189,21 @@ static int at91_reset_of_probe(struct platform_device
*pdev)
return -ENODEV;
}
- for_each_matching_node(np, at91_ramc_of_match) {
- at91_ramc_base[idx] = of_iomap(np, 0);
- if (!at91_ramc_base[idx]) {
- dev_err(pdev-dev, Could not map ram controller
address\n);
- return -ENODEV;
+ match = of_match_node(at91_reset_of_match, pdev-dev.of_node);
+ at91_restart_nb.notifier_call = match-data;
+
+ if (match-data != sama5d3_restart) {
+ /* we need to shutdown the ddr controller, so get ramc base */
+ for_each_matching_node(np, at91_ramc_of_match) {
+ at91_ramc_base[idx] = of_iomap(np, 0);
+ if (!at91_ramc_base[idx]) {
+ dev_err(pdev-dev, Could not map ram
controller address\n);
+ return -ENODEV;
+ }
+ idx++;
}
- idx++;
}
- match = of_match_node(at91_reset_of_match, pdev-dev.of_node);
- at91_restart_nb.notifier_call = match-data;
return register_restart_handler(at91_restart_nb);
}
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] media: atmel-isi: move configure_geometry() to start_streaming()
As in set_fmt() function we only need to know which format is been set,
we don't need to access the ISI hardware in this moment.
So move the configure_geometry(), which access the ISI hardware, to
start_streaming() will make code more consistent and simpler.
Signed-off-by: Josh Wu
---
drivers/media/platform/soc_camera/atmel-isi.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 8bc40ca..b01086d 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -390,6 +390,11 @@ static int start_streaming(struct vb2_queue *vq, unsigned
int count)
/* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
+ ret = configure_geometry(isi, icd->user_width, icd->user_height,
+ icd->current_fmt->code);
+ if (ret < 0)
+ return ret;
+
spin_lock_irq(>lock);
/* Clear any pending interrupt */
isi_readl(isi, ISI_STATUS);
@@ -477,8 +482,6 @@ static int isi_camera_init_videobuf(struct vb2_queue *q,
static int isi_camera_set_fmt(struct soc_camera_device *icd,
struct v4l2_format *f)
{
- struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
- struct atmel_isi *isi = ici->priv;
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
const struct soc_camera_format_xlate *xlate;
struct v4l2_pix_format *pix = >fmt.pix;
@@ -511,16 +514,6 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd,
if (mf->code != xlate->code)
return -EINVAL;
- /* Enable PM and peripheral clock before operate isi registers */
- pm_runtime_get_sync(ici->v4l2_dev.dev);
-
- ret = configure_geometry(isi, pix->width, pix->height, xlate->code);
-
- pm_runtime_put(ici->v4l2_dev.dev);
-
- if (ret < 0)
- return ret;
-
pix->width = mf->width;
pix->height = mf->height;
pix->field = mf->field;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] media: atmel-isi: setup the ISI_CFG2 register directly
In the function configure_geometry(), we will setup the ISI CFG2
according to the sensor output format.
It make no sense to just read back the CFG2 register and just set part
of it.
So just set up this register directly makes things simpler.
Currently only support YUV format from camera sensor.
Signed-off-by: Josh Wu
---
drivers/media/platform/soc_camera/atmel-isi.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 9070172..8bc40ca 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -105,24 +105,25 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
static int configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
- u32 cfg2, cr;
+ u32 cfg2;
+ /* According to sensor's output format to set cfg2 */
switch (code) {
/* YUV, including grey */
case MEDIA_BUS_FMT_Y8_1X8:
- cr = ISI_CFG2_GRAYSCALE;
+ cfg2 = ISI_CFG2_GRAYSCALE;
break;
case MEDIA_BUS_FMT_VYUY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_3;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
case MEDIA_BUS_FMT_UYVY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_2;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_2;
break;
case MEDIA_BUS_FMT_YVYU8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_1;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_1;
break;
case MEDIA_BUS_FMT_YUYV8_2X8:
- cr = ISI_CFG2_YCC_SWAP_DEFAULT;
+ cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
default:
@@ -130,17 +131,10 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
-
- cfg2 = isi_readl(isi, ISI_CFG2);
- /* Set YCC swap mode */
- cfg2 &= ~ISI_CFG2_YCC_SWAP_MODE_MASK;
- cfg2 |= cr;
/* Set width */
- cfg2 &= ~(ISI_CFG2_IM_HSIZE_MASK);
cfg2 |= ((width - 1) << ISI_CFG2_IM_HSIZE_OFFSET) &
ISI_CFG2_IM_HSIZE_MASK;
/* Set height */
- cfg2 &= ~(ISI_CFG2_IM_VSIZE_MASK);
cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
& ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] media: atmel-isi: increase timeout to disable/enable isi
If ISI is working on a 1024x768 or higher resolution, it needs longer time to disable ISI. So this patch will increase timeout to 500ms. Signed-off-by: Josh Wu --- drivers/media/platform/soc_camera/atmel-isi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c index 1482af2..354b7db 100644 --- a/drivers/media/platform/soc_camera/atmel-isi.c +++ b/drivers/media/platform/soc_camera/atmel-isi.c @@ -219,7 +219,7 @@ static int atmel_isi_wait_status(struct atmel_isi *isi, int wait_reset) } timeout = wait_for_completion_timeout(>complete, - msecs_to_jiffies(100)); + msecs_to_jiffies(500)); if (timeout == 0) return -ETIMEDOUT; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] media: atmel-isi: setup the ISI_CFG2 register directly
In the function configure_geometry(), we will setup the ISI CFG2
according to the sensor output format.
It make no sense to just read back the CFG2 register and just set part
of it.
So just set up this register directly makes things simpler.
Currently only support YUV format from camera sensor.
Signed-off-by: Josh Wu josh...@atmel.com
---
drivers/media/platform/soc_camera/atmel-isi.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 9070172..8bc40ca 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -105,24 +105,25 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
static int configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, u32 code)
{
- u32 cfg2, cr;
+ u32 cfg2;
+ /* According to sensor's output format to set cfg2 */
switch (code) {
/* YUV, including grey */
case MEDIA_BUS_FMT_Y8_1X8:
- cr = ISI_CFG2_GRAYSCALE;
+ cfg2 = ISI_CFG2_GRAYSCALE;
break;
case MEDIA_BUS_FMT_VYUY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_3;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_3;
break;
case MEDIA_BUS_FMT_UYVY8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_2;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_2;
break;
case MEDIA_BUS_FMT_YVYU8_2X8:
- cr = ISI_CFG2_YCC_SWAP_MODE_1;
+ cfg2 = ISI_CFG2_YCC_SWAP_MODE_1;
break;
case MEDIA_BUS_FMT_YUYV8_2X8:
- cr = ISI_CFG2_YCC_SWAP_DEFAULT;
+ cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT;
break;
/* RGB, TODO */
default:
@@ -130,17 +131,10 @@ static int configure_geometry(struct atmel_isi *isi, u32
width,
}
isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
-
- cfg2 = isi_readl(isi, ISI_CFG2);
- /* Set YCC swap mode */
- cfg2 = ~ISI_CFG2_YCC_SWAP_MODE_MASK;
- cfg2 |= cr;
/* Set width */
- cfg2 = ~(ISI_CFG2_IM_HSIZE_MASK);
cfg2 |= ((width - 1) ISI_CFG2_IM_HSIZE_OFFSET)
ISI_CFG2_IM_HSIZE_MASK;
/* Set height */
- cfg2 = ~(ISI_CFG2_IM_VSIZE_MASK);
cfg2 |= ((height - 1) ISI_CFG2_IM_VSIZE_OFFSET)
ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] media: atmel-isi: increase timeout to disable/enable isi
If ISI is working on a 1024x768 or higher resolution, it needs longer time to disable ISI. So this patch will increase timeout to 500ms. Signed-off-by: Josh Wu josh...@atmel.com --- drivers/media/platform/soc_camera/atmel-isi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c index 1482af2..354b7db 100644 --- a/drivers/media/platform/soc_camera/atmel-isi.c +++ b/drivers/media/platform/soc_camera/atmel-isi.c @@ -219,7 +219,7 @@ static int atmel_isi_wait_status(struct atmel_isi *isi, int wait_reset) } timeout = wait_for_completion_timeout(isi-complete, - msecs_to_jiffies(100)); + msecs_to_jiffies(500)); if (timeout == 0) return -ETIMEDOUT; -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] media: atmel-isi: move configure_geometry() to start_streaming()
As in set_fmt() function we only need to know which format is been set,
we don't need to access the ISI hardware in this moment.
So move the configure_geometry(), which access the ISI hardware, to
start_streaming() will make code more consistent and simpler.
Signed-off-by: Josh Wu josh...@atmel.com
---
drivers/media/platform/soc_camera/atmel-isi.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 8bc40ca..b01086d 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -390,6 +390,11 @@ static int start_streaming(struct vb2_queue *vq, unsigned
int count)
/* Disable all interrupts */
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
+ ret = configure_geometry(isi, icd-user_width, icd-user_height,
+ icd-current_fmt-code);
+ if (ret 0)
+ return ret;
+
spin_lock_irq(isi-lock);
/* Clear any pending interrupt */
isi_readl(isi, ISI_STATUS);
@@ -477,8 +482,6 @@ static int isi_camera_init_videobuf(struct vb2_queue *q,
static int isi_camera_set_fmt(struct soc_camera_device *icd,
struct v4l2_format *f)
{
- struct soc_camera_host *ici = to_soc_camera_host(icd-parent);
- struct atmel_isi *isi = ici-priv;
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
const struct soc_camera_format_xlate *xlate;
struct v4l2_pix_format *pix = f-fmt.pix;
@@ -511,16 +514,6 @@ static int isi_camera_set_fmt(struct soc_camera_device
*icd,
if (mf-code != xlate-code)
return -EINVAL;
- /* Enable PM and peripheral clock before operate isi registers */
- pm_runtime_get_sync(ici-v4l2_dev.dev);
-
- ret = configure_geometry(isi, pix-width, pix-height, xlate-code);
-
- pm_runtime_put(ici-v4l2_dev.dev);
-
- if (ret 0)
- return ret;
-
pix-width = mf-width;
pix-height = mf-height;
pix-field = mf-field;
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
