From: Chen-Yu Tsai <[email protected]>
The A10/A20 Allwinner SoCs have two camera sensor interface blocks,
named CSI0 and CSI1. The two have the same register layouts with
slightly different features:
- CSI0 has an image signal processor (ISP); CSI1 doesn't
- CSI0 can support up to four separate channels under CCIR656;
CSI1 can only support one
- CSI0 can support up to 16-bit wide bus with YUV422;
CSI1 can support up to 24-bit wide bus with YUV444
For now the driver doesn't support wide busses, nor CCIR656. So the
only relevant difference is whether a clock needs to be taken and
enabled for the ISP.
Add structs to record the differences, tie them to the compatible
strings, and deal with the ISP clock. Support for the new CSI1
hardware block is added as well.
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
.../platform/sunxi/sun4i-csi/sun4i_csi.c | 35 ++++++++++++++++---
.../platform/sunxi/sun4i-csi/sun4i_csi.h | 2 ++
2 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
index b8b07c1de2a8..be2466930a49 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
@@ -29,6 +29,12 @@
#include "sun4i_csi.h"
+struct sun4i_csi_traits {
+ unsigned int channels;
+ unsigned int max_width;
+ bool has_isp;
+};
+
static const struct media_entity_operations sun4i_csi_video_entity_ops = {
.link_validate = v4l2_subdev_link_validate,
};
@@ -156,6 +162,10 @@ static int sun4i_csi_probe(struct platform_device *pdev)
subdev = &csi->subdev;
vdev = &csi->vdev;
+ csi->traits = of_device_get_match_data(&pdev->dev);
+ if (!csi->traits)
+ return -EINVAL;
+
/*
* On Allwinner SoCs, some high memory bandwidth devices do DMA
* directly over the memory bus (called MBUS), instead of the
@@ -199,10 +209,12 @@ static int sun4i_csi_probe(struct platform_device *pdev)
return PTR_ERR(csi->bus_clk);
}
- csi->isp_clk = devm_clk_get(&pdev->dev, "isp");
- if (IS_ERR(csi->isp_clk)) {
- dev_err(&pdev->dev, "Couldn't get our ISP clock\n");
- return PTR_ERR(csi->isp_clk);
+ if (csi->traits->has_isp) {
+ csi->isp_clk = devm_clk_get(&pdev->dev, "isp");
+ if (IS_ERR(csi->isp_clk)) {
+ dev_err(&pdev->dev, "Couldn't get our ISP clock\n");
+ return PTR_ERR(csi->isp_clk);
+ }
}
csi->ram_clk = devm_clk_get(&pdev->dev, "ram");
@@ -280,8 +292,21 @@ static int sun4i_csi_remove(struct platform_device *pdev)
return 0;
}
+struct sun4i_csi_traits sun4i_a10_csi1_traits = {
+ .channels = 1,
+ .max_width = 24,
+ .has_isp = false,
+};
+
+struct sun4i_csi_traits sun7i_a20_csi0_traits = {
+ .channels = 4,
+ .max_width = 16,
+ .has_isp = true,
+};
+
static const struct of_device_id sun4i_csi_of_match[] = {
- { .compatible = "allwinner,sun7i-a20-csi0" },
+ { .compatible = "allwinner,sun4i-a10-csi1", .data = &sun4i_a10_csi1_traits },
+ { .compatible = "allwinner,sun7i-a20-csi0", .data = &sun7i_a20_csi0_traits },
{ /* Sentinel */ }
};
MODULE_DEVICE_TABLE(of, sun4i_csi_of_match);
diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
index 88d39b3554c4..0f67ff652c2e 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
@@ -108,6 +108,8 @@ struct sun4i_csi {
/* Device resources */
struct device *dev;
+ const struct sun4i_csi_traits *traits;
+
void __iomem *regs;
struct clk *bus_clk;
struct clk *isp_clk;
--
2.24.0
Hi Chen-Yu,
Thanks for the patchset.
On Mon, Dec 16, 2019 at 12:59:16AM +0800, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <[email protected]>
>
> The A10/A20 Allwinner SoCs have two camera sensor interface blocks,
> named CSI0 and CSI1. The two have the same register layouts with
> slightly different features:
>
> - CSI0 has an image signal processor (ISP); CSI1 doesn't
>
> - CSI0 can support up to four separate channels under CCIR656;
> CSI1 can only support one
>
> - CSI0 can support up to 16-bit wide bus with YUV422;
> CSI1 can support up to 24-bit wide bus with YUV444
>
> For now the driver doesn't support wide busses, nor CCIR656. So the
> only relevant difference is whether a clock needs to be taken and
> enabled for the ISP.
>
> Add structs to record the differences, tie them to the compatible
> strings, and deal with the ISP clock. Support for the new CSI1
> hardware block is added as well.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
> ---
> .../platform/sunxi/sun4i-csi/sun4i_csi.c | 35 ++++++++++++++++---
> .../platform/sunxi/sun4i-csi/sun4i_csi.h | 2 ++
> 2 files changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> index b8b07c1de2a8..be2466930a49 100644
> --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> @@ -29,6 +29,12 @@
>
> #include "sun4i_csi.h"
>
> +struct sun4i_csi_traits {
> + unsigned int channels;
> + unsigned int max_width;
> + bool has_isp;
> +};
> +
> static const struct media_entity_operations sun4i_csi_video_entity_ops = {
> .link_validate = v4l2_subdev_link_validate,
> };
> @@ -156,6 +162,10 @@ static int sun4i_csi_probe(struct platform_device *pdev)
> subdev = &csi->subdev;
> vdev = &csi->vdev;
>
> + csi->traits = of_device_get_match_data(&pdev->dev);
> + if (!csi->traits)
> + return -EINVAL;
> +
> /*
> * On Allwinner SoCs, some high memory bandwidth devices do DMA
> * directly over the memory bus (called MBUS), instead of the
> @@ -199,10 +209,12 @@ static int sun4i_csi_probe(struct platform_device *pdev)
> return PTR_ERR(csi->bus_clk);
> }
>
> - csi->isp_clk = devm_clk_get(&pdev->dev, "isp");
> - if (IS_ERR(csi->isp_clk)) {
> - dev_err(&pdev->dev, "Couldn't get our ISP clock\n");
> - return PTR_ERR(csi->isp_clk);
> + if (csi->traits->has_isp) {
> + csi->isp_clk = devm_clk_get(&pdev->dev, "isp");
> + if (IS_ERR(csi->isp_clk)) {
> + dev_err(&pdev->dev, "Couldn't get our ISP clock\n");
> + return PTR_ERR(csi->isp_clk);
> + }
> }
>
> csi->ram_clk = devm_clk_get(&pdev->dev, "ram");
> @@ -280,8 +292,21 @@ static int sun4i_csi_remove(struct platform_device *pdev)
> return 0;
> }
>
> +struct sun4i_csi_traits sun4i_a10_csi1_traits = {
> + .channels = 1,
> + .max_width = 24,
> + .has_isp = false,
> +};
> +
> +struct sun4i_csi_traits sun7i_a20_csi0_traits = {
These two should be static const, right?
> + .channels = 4,
> + .max_width = 16,
> + .has_isp = true,
> +};
> +
> static const struct of_device_id sun4i_csi_of_match[] = {
> - { .compatible = "allwinner,sun7i-a20-csi0" },
> + { .compatible = "allwinner,sun4i-a10-csi1", .data = &sun4i_a10_csi1_traits },
> + { .compatible = "allwinner,sun7i-a20-csi0", .data = &sun7i_a20_csi0_traits },
> { /* Sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, sun4i_csi_of_match);
> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
> index 88d39b3554c4..0f67ff652c2e 100644
> --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
> @@ -108,6 +108,8 @@ struct sun4i_csi {
> /* Device resources */
> struct device *dev;
>
> + const struct sun4i_csi_traits *traits;
> +
> void __iomem *regs;
> struct clk *bus_clk;
> struct clk *isp_clk;
--
Kind regards,
Sakari Ailus
On Thu, Jan 2, 2020 at 7:33 PM Sakari Ailus
<[email protected]> wrote:
>
> Hi Chen-Yu,
>
> Thanks for the patchset.
>
> On Mon, Dec 16, 2019 at 12:59:16AM +0800, Chen-Yu Tsai wrote:
> > From: Chen-Yu Tsai <[email protected]>
> >
> > The A10/A20 Allwinner SoCs have two camera sensor interface blocks,
> > named CSI0 and CSI1. The two have the same register layouts with
> > slightly different features:
> >
> > - CSI0 has an image signal processor (ISP); CSI1 doesn't
> >
> > - CSI0 can support up to four separate channels under CCIR656;
> > CSI1 can only support one
> >
> > - CSI0 can support up to 16-bit wide bus with YUV422;
> > CSI1 can support up to 24-bit wide bus with YUV444
> >
> > For now the driver doesn't support wide busses, nor CCIR656. So the
> > only relevant difference is whether a clock needs to be taken and
> > enabled for the ISP.
> >
> > Add structs to record the differences, tie them to the compatible
> > strings, and deal with the ISP clock. Support for the new CSI1
> > hardware block is added as well.
> >
> > Signed-off-by: Chen-Yu Tsai <[email protected]>
> > ---
> > .../platform/sunxi/sun4i-csi/sun4i_csi.c | 35 ++++++++++++++++---
> > .../platform/sunxi/sun4i-csi/sun4i_csi.h | 2 ++
> > 2 files changed, 32 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> > index b8b07c1de2a8..be2466930a49 100644
> > --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> > +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> > @@ -29,6 +29,12 @@
> >
> > #include "sun4i_csi.h"
> >
> > +struct sun4i_csi_traits {
> > + unsigned int channels;
> > + unsigned int max_width;
> > + bool has_isp;
> > +};
> > +
> > static const struct media_entity_operations sun4i_csi_video_entity_ops = {
> > .link_validate = v4l2_subdev_link_validate,
> > };
> > @@ -156,6 +162,10 @@ static int sun4i_csi_probe(struct platform_device *pdev)
> > subdev = &csi->subdev;
> > vdev = &csi->vdev;
> >
> > + csi->traits = of_device_get_match_data(&pdev->dev);
> > + if (!csi->traits)
> > + return -EINVAL;
> > +
> > /*
> > * On Allwinner SoCs, some high memory bandwidth devices do DMA
> > * directly over the memory bus (called MBUS), instead of the
> > @@ -199,10 +209,12 @@ static int sun4i_csi_probe(struct platform_device *pdev)
> > return PTR_ERR(csi->bus_clk);
> > }
> >
> > - csi->isp_clk = devm_clk_get(&pdev->dev, "isp");
> > - if (IS_ERR(csi->isp_clk)) {
> > - dev_err(&pdev->dev, "Couldn't get our ISP clock\n");
> > - return PTR_ERR(csi->isp_clk);
> > + if (csi->traits->has_isp) {
> > + csi->isp_clk = devm_clk_get(&pdev->dev, "isp");
> > + if (IS_ERR(csi->isp_clk)) {
> > + dev_err(&pdev->dev, "Couldn't get our ISP clock\n");
> > + return PTR_ERR(csi->isp_clk);
> > + }
> > }
> >
> > csi->ram_clk = devm_clk_get(&pdev->dev, "ram");
> > @@ -280,8 +292,21 @@ static int sun4i_csi_remove(struct platform_device *pdev)
> > return 0;
> > }
> >
> > +struct sun4i_csi_traits sun4i_a10_csi1_traits = {
> > + .channels = 1,
> > + .max_width = 24,
> > + .has_isp = false,
> > +};
> > +
> > +struct sun4i_csi_traits sun7i_a20_csi0_traits = {
>
> These two should be static const, right?
You're right. I'll wait for the remaining reviews before sending v2.
ChenYu
> > + .channels = 4,
> > + .max_width = 16,
> > + .has_isp = true,
> > +};
> > +
> > static const struct of_device_id sun4i_csi_of_match[] = {
> > - { .compatible = "allwinner,sun7i-a20-csi0" },
> > + { .compatible = "allwinner,sun4i-a10-csi1", .data = &sun4i_a10_csi1_traits },
> > + { .compatible = "allwinner,sun7i-a20-csi0", .data = &sun7i_a20_csi0_traits },
> > { /* Sentinel */ }
> > };
> > MODULE_DEVICE_TABLE(of, sun4i_csi_of_match);
> > diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
> > index 88d39b3554c4..0f67ff652c2e 100644
> > --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
> > +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
> > @@ -108,6 +108,8 @@ struct sun4i_csi {
> > /* Device resources */
> > struct device *dev;
> >
> > + const struct sun4i_csi_traits *traits;
> > +
> > void __iomem *regs;
> > struct clk *bus_clk;
> > struct clk *isp_clk;
>
> --
> Kind regards,
>
> Sakari Ailus