Re: [PATCH v4 4/4] smiapp: Use v4l2_of_alloc_parse_endpoint()
Hi Prabhakar,
Thank you for the review.
On Fri, Apr 10, 2015 at 10:54:08PM +0100, Lad, Prabhakar wrote:
> Hi Sakari,
>
> Thanks for the patch.
>
> On Thu, Apr 9, 2015 at 10:25 PM, Sakari Ailus wrote:
> > Instead of parsing the link-frequencies property in the driver, let
> > v4l2_of_alloc_parse_endpoint() do it.
> >
> > Signed-off-by: Sakari Ailus
> > Acked-by: Laurent Pinchart
> > ---
> > drivers/media/i2c/smiapp/smiapp-core.c | 40
> >
> > 1 file changed, 20 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/media/i2c/smiapp/smiapp-core.c
> > b/drivers/media/i2c/smiapp/smiapp-core.c
> > index 557f25d..4a2e8d3 100644
> > --- a/drivers/media/i2c/smiapp/smiapp-core.c
> > +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> > @@ -2975,9 +2975,9 @@ static int smiapp_resume(struct device *dev)
> > static struct smiapp_platform_data *smiapp_get_pdata(struct device *dev)
> > {
> > struct smiapp_platform_data *pdata;
> > - struct v4l2_of_endpoint bus_cfg;
> > + struct v4l2_of_endpoint *bus_cfg;
> > struct device_node *ep;
> > - uint32_t asize;
> > + int i;
> > int rval;
> >
> > if (!dev->of_node)
> > @@ -2987,13 +2987,17 @@ static struct smiapp_platform_data
> > *smiapp_get_pdata(struct device *dev)
> > if (!ep)
> > return NULL;
> >
> > + bus_cfg = v4l2_of_alloc_parse_endpoint(ep);
> > + if (IS_ERR(bus_cfg)) {
> > + rval = PTR_ERR(bus_cfg);
>
> this assignment is not required.
>
> Apart from that the patch looks good.
>
> Reviewed-by: Lad, Prabhakar
Good point. I'll remove it.
There's another case of the same in the function, I'll send a separate patch
on that. I'll send a pull request on these soonish.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] smiapp: Use v4l2_of_alloc_parse_endpoint()
Hi Sakari,
Thanks for the patch.
On Thu, Apr 9, 2015 at 10:25 PM, Sakari Ailus wrote:
> Instead of parsing the link-frequencies property in the driver, let
> v4l2_of_alloc_parse_endpoint() do it.
>
> Signed-off-by: Sakari Ailus
> Acked-by: Laurent Pinchart
> ---
> drivers/media/i2c/smiapp/smiapp-core.c | 40
>
> 1 file changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c
> b/drivers/media/i2c/smiapp/smiapp-core.c
> index 557f25d..4a2e8d3 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -2975,9 +2975,9 @@ static int smiapp_resume(struct device *dev)
> static struct smiapp_platform_data *smiapp_get_pdata(struct device *dev)
> {
> struct smiapp_platform_data *pdata;
> - struct v4l2_of_endpoint bus_cfg;
> + struct v4l2_of_endpoint *bus_cfg;
> struct device_node *ep;
> - uint32_t asize;
> + int i;
> int rval;
>
> if (!dev->of_node)
> @@ -2987,13 +2987,17 @@ static struct smiapp_platform_data
> *smiapp_get_pdata(struct device *dev)
> if (!ep)
> return NULL;
>
> + bus_cfg = v4l2_of_alloc_parse_endpoint(ep);
> + if (IS_ERR(bus_cfg)) {
> + rval = PTR_ERR(bus_cfg);
this assignment is not required.
Apart from that the patch looks good.
Reviewed-by: Lad, Prabhakar
Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 4/4] smiapp: Use v4l2_of_alloc_parse_endpoint()
Instead of parsing the link-frequencies property in the driver, let
v4l2_of_alloc_parse_endpoint() do it.
Signed-off-by: Sakari Ailus
Acked-by: Laurent Pinchart
---
drivers/media/i2c/smiapp/smiapp-core.c | 40
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/media/i2c/smiapp/smiapp-core.c
b/drivers/media/i2c/smiapp/smiapp-core.c
index 557f25d..4a2e8d3 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2975,9 +2975,9 @@ static int smiapp_resume(struct device *dev)
static struct smiapp_platform_data *smiapp_get_pdata(struct device *dev)
{
struct smiapp_platform_data *pdata;
- struct v4l2_of_endpoint bus_cfg;
+ struct v4l2_of_endpoint *bus_cfg;
struct device_node *ep;
- uint32_t asize;
+ int i;
int rval;
if (!dev->of_node)
@@ -2987,13 +2987,17 @@ static struct smiapp_platform_data
*smiapp_get_pdata(struct device *dev)
if (!ep)
return NULL;
+ bus_cfg = v4l2_of_alloc_parse_endpoint(ep);
+ if (IS_ERR(bus_cfg)) {
+ rval = PTR_ERR(bus_cfg);
+ goto out_err;
+ }
+
pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata)
goto out_err;
- v4l2_of_parse_endpoint(ep, &bus_cfg);
-
- switch (bus_cfg.bus_type) {
+ switch (bus_cfg->bus_type) {
case V4L2_MBUS_CSI2:
pdata->csi_signalling_mode = SMIAPP_CSI_SIGNALLING_MODE_CSI2;
break;
@@ -3002,7 +3006,7 @@ static struct smiapp_platform_data
*smiapp_get_pdata(struct device *dev)
goto out_err;
}
- pdata->lanes = bus_cfg.bus.mipi_csi2.num_data_lanes;
+ pdata->lanes = bus_cfg->bus.mipi_csi2.num_data_lanes;
dev_dbg(dev, "lanes %u\n", pdata->lanes);
/* xshutdown GPIO is optional */
@@ -3022,34 +3026,30 @@ static struct smiapp_platform_data
*smiapp_get_pdata(struct device *dev)
dev_dbg(dev, "reset %d, nvm %d, clk %d, csi %d\n", pdata->xshutdown,
pdata->nvm_size, pdata->ext_clk, pdata->csi_signalling_mode);
- rval = of_get_property(ep, "link-frequencies", &asize) ? 0 : -ENOENT;
- if (rval) {
- dev_warn(dev, "can't get link-frequencies array size\n");
+ if (!bus_cfg->nr_of_link_frequencies) {
+ dev_warn(dev, "no link frequencies defined\n");
goto out_err;
}
- pdata->op_sys_clock = devm_kzalloc(dev, asize, GFP_KERNEL);
+ pdata->op_sys_clock = devm_kcalloc(
+ dev, bus_cfg->nr_of_link_frequencies + 1 /* guardian */,
+ sizeof(*pdata->op_sys_clock), GFP_KERNEL);
if (!pdata->op_sys_clock) {
rval = -ENOMEM;
goto out_err;
}
- asize /= sizeof(*pdata->op_sys_clock);
- rval = of_property_read_u64_array(
- ep, "link-frequencies", pdata->op_sys_clock, asize);
- if (rval) {
- dev_warn(dev, "can't get link-frequencies\n");
- goto out_err;
+ for (i = 0; i < bus_cfg->nr_of_link_frequencies; i++) {
+ pdata->op_sys_clock[i] = bus_cfg->link_frequencies[i];
+ dev_dbg(dev, "freq %d: %lld\n", i, pdata->op_sys_clock[i]);
}
- for (; asize > 0; asize--)
- dev_dbg(dev, "freq %d: %lld\n", asize - 1,
- pdata->op_sys_clock[asize - 1]);
-
+ v4l2_of_free_endpoint(bus_cfg);
of_node_put(ep);
return pdata;
out_err:
+ v4l2_of_free_endpoint(bus_cfg);
of_node_put(ep);
return NULL;
}
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
