| Message ID | 1360128584-23167-1-git-send-email-sachin.kamat@linaro.org |
|---|---|
| State | Accepted |
| Headers | show |
Hi Sylwester, On Wednesday, 6 February 2013, Sachin Kamat <sachin.kamat@linaro.org> wrote: > This patch adds device tree based discovery support to G2D driver > > Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org> > --- > Based on for_v3.9 branch of below tree: > git://linuxtv.org/snawrocki/samsung.git > > Changes since v1: > * Addressed review comments from Sylwester <s.nawrocki@samsung.com>. > * Modified the compatible string as per the discussions at [1]. > [1] https://patchwork1.kernel.org/patch/2045821/ > Does this patch look good?
On 02/12/2013 06:30 PM, Sachin Kamat wrote: > > Hi Sylwester, > > On Wednesday, 6 February 2013, Sachin Kamat <sachin.kamat@linaro.org> wrote: >> This patch adds device tree based discovery support to G2D driver >> >> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org> >> --- >> Based on for_v3.9 branch of below tree: >> git://linuxtv.org/snawrocki/samsung.git >> >> Changes since v1: >> * Addressed review comments from Sylwester <s.nawrocki@samsung.com>. >> * Modified the compatible string as per the discussions at [1]. >> [1] https://patchwork1.kernel.org/patch/2045821/ >> > > Does this patch look good? It looks OK to me. I've sent a pull request including it, but it may happen it ends up only in 3.10. I tried to test this patch today and I had to correct some clock definitions in the common clock API driver [1]. And we already have quite a few fixes to that patch series. Shouldn't you also provide a patch adding related OF_DEV_AUXDATA entry ? How did you test this one ? When the new clocks driver gets merged (I guess it happens only in 3.10) I'd like to have the media devices' clock names cleaned up, instead of names like: {"sclk_fimg2d", "fimg2d"}, {"sclk_fimc", "fimc"}, {"sclk_fimd"/"fimd"}, in clock-names property we could have common names, e.g. { "sclk", "gate" }. This could simplify a bit subsystems like devfreq. Also I noticed there are some issues caused by splitting mux + div + gate clocks into 3 different clocks. One solution to this might be to use the new composite clock type. [1] http://www.spinics.net/lists/arm-kernel/msg214149.html
On Thursday, 14 February 2013, Sylwester Nawrocki < sylvester.nawrocki@gmail.com> wrote: > On 02/12/2013 06:30 PM, Sachin Kamat wrote: >> >> Hi Sylwester, >> >> On Wednesday, 6 February 2013, Sachin Kamat <sachin.kamat@linaro.org> wrote: >>> >>> This patch adds device tree based discovery support to G2D driver >>> >>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org> >>> --- >>> Based on for_v3.9 branch of below tree: >>> git://linuxtv.org/snawrocki/samsung.git >>> >>> Changes since v1: >>> * Addressed review comments from Sylwester <s.nawrocki@samsung.com>. >>> * Modified the compatible string as per the discussions at [1]. >>> [1] https://patchwork1.kernel.org/patch/2045821/ >>> >> >> Does this patch look good? > > It looks OK to me. I've sent a pull request including it, but it may > happen it ends up only in 3.10. Thanks. Hope it gets picked for 3.9 itself. > > I tried to test this patch today and I had to correct some clock > definitions in the common clock API driver [1]. And we already have > quite a few fixes to that patch series. > > Shouldn't you also provide a patch adding related OF_DEV_AUXDATA entry ? > How did you test this one ? I tested this without the common clock patches, with the mainline tree. It did not require any auxdata entry. > > When the new clocks driver gets merged (I guess it happens only in 3.10) > I'd like to have the media devices' clock names cleaned up, instead of > names like: {"sclk_fimg2d", "fimg2d"}, {"sclk_fimc", "fimc"}, > {"sclk_fimd"/"fimd"}, in clock-names property we could have common names, > e.g. { "sclk", "gate" }. This could simplify a bit subsystems like devfreq. Yes. That makes sense. > > Also I noticed there are some issues caused by splitting mux + div + gate > clocks into 3 different clocks. One solution to this might be to use the > new composite clock type. Ok. > > [1] http://www.spinics.net/lists/arm-kernel/msg214149.html >
diff --git a/drivers/media/platform/s5p-g2d/g2d.c b/drivers/media/platform/s5p-g2d/g2d.c index 7e41529..6923be2 100644 --- a/drivers/media/platform/s5p-g2d/g2d.c +++ b/drivers/media/platform/s5p-g2d/g2d.c @@ -18,6 +18,7 @@ #include <linux/slab.h> #include <linux/clk.h> #include <linux/interrupt.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <media/v4l2-mem2mem.h> @@ -695,11 +696,14 @@ static struct v4l2_m2m_ops g2d_m2m_ops = { .unlock = g2d_unlock, }; +static const struct of_device_id exynos_g2d_match[]; + static int g2d_probe(struct platform_device *pdev) { struct g2d_dev *dev; struct video_device *vfd; struct resource *res; + const struct of_device_id *of_id; int ret = 0; dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL); @@ -796,7 +800,17 @@ static int g2d_probe(struct platform_device *pdev) } def_frame.stride = (def_frame.width * def_frame.fmt->depth) >> 3; - dev->variant = g2d_get_drv_data(pdev); + + if (!pdev->dev.of_node) { + dev->variant = g2d_get_drv_data(pdev); + } else { + of_id = of_match_node(exynos_g2d_match, pdev->dev.of_node); + if (!of_id) { + ret = -ENODEV; + goto unreg_video_dev; + } + dev->variant = (struct g2d_variant *)of_id->data; + } return 0; @@ -837,13 +851,25 @@ static int g2d_remove(struct platform_device *pdev) } static struct g2d_variant g2d_drvdata_v3x = { - .hw_rev = TYPE_G2D_3X, + .hw_rev = TYPE_G2D_3X, /* Revision 3.0 for S5PV210 and Exynos4210 */ }; static struct g2d_variant g2d_drvdata_v4x = { .hw_rev = TYPE_G2D_4X, /* Revision 4.1 for Exynos4X12 and Exynos5 */ }; +static const struct of_device_id exynos_g2d_match[] = { + { + .compatible = "samsung,s5pv210-g2d", + .data = &g2d_drvdata_v3x, + }, { + .compatible = "samsung,exynos4212-g2d", + .data = &g2d_drvdata_v4x, + }, + {}, +}; +MODULE_DEVICE_TABLE(of, exynos_g2d_match); + static struct platform_device_id g2d_driver_ids[] = { { .name = "s5p-g2d", @@ -863,6 +889,7 @@ static struct platform_driver g2d_pdrv = { .driver = { .name = G2D_NAME, .owner = THIS_MODULE, + .of_match_table = exynos_g2d_match, }, };
This patch adds device tree based discovery support to G2D driver Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org> --- Based on for_v3.9 branch of below tree: git://linuxtv.org/snawrocki/samsung.git Changes since v1: * Addressed review comments from Sylwester <s.nawrocki@samsung.com>. * Modified the compatible string as per the discussions at [1]. [1] https://patchwork1.kernel.org/patch/2045821/ --- drivers/media/platform/s5p-g2d/g2d.c | 31 +++++++++++++++++++++++++++++-- 1 files changed, 29 insertions(+), 2 deletions(-)