Re: [PATCH v3 6/6] media: mt9m111: allow to setup pixclk polarity

2018-11-28 Thread Sakari Ailus
On Wed, Nov 28, 2018 at 09:29:01AM +0100, Marco Felsch wrote:
> On 18-11-27 23:15, Sakari Ailus wrote:
> > On Tue, Nov 27, 2018 at 11:02:53AM +0100, Marco Felsch wrote:
> > > From: Enrico Scholz 
> > > 
> > > The chip can be configured to output data transitions on the
> > > rising or falling edge of PIXCLK (Datasheet R58:1[9]), default is on the
> > > falling edge.
> > > 
> > > Parsing the fw-node is made in a subfunction to bundle all (future)
> > > dt-parsing / fw-parsing stuff.
> > > 
> > > Signed-off-by: Enrico Scholz 
> > > (m.grzesc...@pengutronix.de: Fix inverting clock. INV_PIX_CLOCK bit is set
> > > per default. Set bit to 0 (enable mask bit without value) to enable
> > > falling edge sampling.)
> > > Signed-off-by: Michael Grzeschik 
> > > (m.fel...@pengutronix.de: use fwnode helpers)
> > > (m.fel...@pengutronix.de: mv fw parsing into own function)
> > > (m.fel...@pengutronix.de: adapt commit msg)
> > > Signed-off-by: Marco Felsch 
> > 
> > This one as well:
> 
> Sorry for that, I forget to adapt the Kconfig to often. Thanks for your
> fix.

No worries. I hope we'll have automated compile testing in not too distant
future...

-- 
Sakari Ailus
sakari.ai...@linux.intel.com


Re: [PATCH v3 6/6] media: mt9m111: allow to setup pixclk polarity

2018-11-28 Thread Marco Felsch
On 18-11-27 23:15, Sakari Ailus wrote:
> On Tue, Nov 27, 2018 at 11:02:53AM +0100, Marco Felsch wrote:
> > From: Enrico Scholz 
> > 
> > The chip can be configured to output data transitions on the
> > rising or falling edge of PIXCLK (Datasheet R58:1[9]), default is on the
> > falling edge.
> > 
> > Parsing the fw-node is made in a subfunction to bundle all (future)
> > dt-parsing / fw-parsing stuff.
> > 
> > Signed-off-by: Enrico Scholz 
> > (m.grzesc...@pengutronix.de: Fix inverting clock. INV_PIX_CLOCK bit is set
> > per default. Set bit to 0 (enable mask bit without value) to enable
> > falling edge sampling.)
> > Signed-off-by: Michael Grzeschik 
> > (m.fel...@pengutronix.de: use fwnode helpers)
> > (m.fel...@pengutronix.de: mv fw parsing into own function)
> > (m.fel...@pengutronix.de: adapt commit msg)
> > Signed-off-by: Marco Felsch 
> 
> This one as well:

Sorry for that, I forget to adapt the Kconfig to often. Thanks for your
fix.

Kind regards,
Marco

> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 005fc2bd0d05..902c3cabf44c 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -859,6 +859,7 @@ config VIDEO_MT9M032
>  config VIDEO_MT9M111
>   tristate "mt9m111, mt9m112 and mt9m131 support"
>   depends on I2C && VIDEO_V4L2
> + select V4L2_FWNODE
>   help
> This driver supports MT9M111, MT9M112 and MT9M131 cameras from
> Micron/Aptina
> 
> -- 
> Sakari Ailus
> sakari.ai...@linux.intel.com
> 

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |


Re: [PATCH v3 6/6] media: mt9m111: allow to setup pixclk polarity

2018-11-27 Thread Sakari Ailus
On Tue, Nov 27, 2018 at 11:02:53AM +0100, Marco Felsch wrote:
> From: Enrico Scholz 
> 
> The chip can be configured to output data transitions on the
> rising or falling edge of PIXCLK (Datasheet R58:1[9]), default is on the
> falling edge.
> 
> Parsing the fw-node is made in a subfunction to bundle all (future)
> dt-parsing / fw-parsing stuff.
> 
> Signed-off-by: Enrico Scholz 
> (m.grzesc...@pengutronix.de: Fix inverting clock. INV_PIX_CLOCK bit is set
> per default. Set bit to 0 (enable mask bit without value) to enable
> falling edge sampling.)
> Signed-off-by: Michael Grzeschik 
> (m.fel...@pengutronix.de: use fwnode helpers)
> (m.fel...@pengutronix.de: mv fw parsing into own function)
> (m.fel...@pengutronix.de: adapt commit msg)
> Signed-off-by: Marco Felsch 

This one as well:

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 005fc2bd0d05..902c3cabf44c 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -859,6 +859,7 @@ config VIDEO_MT9M032
 config VIDEO_MT9M111
tristate "mt9m111, mt9m112 and mt9m131 support"
depends on I2C && VIDEO_V4L2
+   select V4L2_FWNODE
help
  This driver supports MT9M111, MT9M112 and MT9M131 cameras from
  Micron/Aptina

-- 
Sakari Ailus
sakari.ai...@linux.intel.com


Re: [PATCH v3 6/6] media: mt9m111: allow to setup pixclk polarity

2018-11-27 Thread Sakari Ailus
On Tue, Nov 27, 2018 at 03:12:29PM +0100, Philipp Zabel wrote:
> Hi Sakari,
> 
> On Tue, 2018-11-27 at 15:50 +0200, Sakari Ailus wrote:
> > Hi Philipp,
> > 
> > On Tue, Nov 27, 2018 at 02:39:27PM +0100, Philipp Zabel wrote:
> > > Hi Sakari,
> > > 
> > > On Tue, 2018-11-27 at 15:19 +0200, Sakari Ailus wrote:
> > > > Hi Marco,
> > > > 
> > > > On Tue, Nov 27, 2018 at 11:02:53AM +0100, Marco Felsch wrote:
> > > > > From: Enrico Scholz 
> > > > > 
> > > > > The chip can be configured to output data transitions on the
> > > > > rising or falling edge of PIXCLK (Datasheet R58:1[9]), default is on 
> > > > > the
> > > > > falling edge.
> > > > > 
> > > > > Parsing the fw-node is made in a subfunction to bundle all (future)
> > > > > dt-parsing / fw-parsing stuff.
> > > > > 
> > > > > Signed-off-by: Enrico Scholz 
> > > > > (m.grzesc...@pengutronix.de: Fix inverting clock. INV_PIX_CLOCK bit 
> > > > > is set
> > > > > per default. Set bit to 0 (enable mask bit without value) to enable
> > > > > falling edge sampling.)
> > > > > Signed-off-by: Michael Grzeschik 
> > > > > (m.fel...@pengutronix.de: use fwnode helpers)
> > > > > (m.fel...@pengutronix.de: mv fw parsing into own function)
> > > > > (m.fel...@pengutronix.de: adapt commit msg)
> > > > > Signed-off-by: Marco Felsch 
> > > > 
> > > > Applied with the following diff:
> > > > 
> > > > diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
> > > > index 2ef332b9b914..b6011bfddde8 100644
> > > > --- a/drivers/media/i2c/mt9m111.c
> > > > +++ b/drivers/media/i2c/mt9m111.c
> > > > @@ -1172,24 +1172,24 @@ static int mt9m111_video_probe(struct 
> > > > i2c_client *client)
> > > >  
> > > >  static int mt9m111_probe_fw(struct i2c_client *client, struct mt9m111 
> > > > *mt9m111)
> > > >  {
> > > > -   struct v4l2_fwnode_endpoint *bus_cfg;
> > > > +   struct v4l2_fwnode_endpoint bus_cfg = {
> > > > +   .bus_type = V4L2_MBUS_PARALLEL
> > > > +   };
> > > > struct fwnode_handle *np;
> > > > -   int ret = 0;
> > > > +   int ret;
> > > >  
> > > > np = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev), 
> > > > NULL);
> > > > if (!np)
> > > > return -EINVAL;
> > > >  
> > > > -   bus_cfg = v4l2_fwnode_endpoint_alloc_parse(np);
> > > > -   if (IS_ERR(bus_cfg)) {
> > > > -   ret = PTR_ERR(bus_cfg);
> > > > +   ret = v4l2_fwnode_endpoint_alloc_parse(np, &bus_cfg);
> > > 
> > > Should that be
> > > 
> > > + ret = v4l2_fwnode_endpoint_parse(np, &bus_cfg);
> > > 
> > > intead?
> > 
> > Could be. I'd expect the driver to need the link frequency at some point
> > after which you'd need the variable size properties anyway. But that's not
> > the case now.
> 
> I don't think the link-frequencies property will be used, this is just a
> parallel device. But Marco chose to use _alloc_parse because of what the
> v4l2_fwnode_endpoint_parse() documentation says:
> 
> /*
>  * NOTE: This function does not parse properties the size of which is variable
>  * without a low fixed limit. Please use v4l2_fwnode_endpoint_alloc_parse() 
> in
>  * new drivers instead.
>  */
> 
> So maybe we want to use v4l2_fwnode_endpoint_alloc_parse() always. There
> is no unnecessary allocation, just a lookup of the non-existing link-
> frequencies property.

There could be other properties in the future.

When I wrote that, I guess I ignored that the link frequency might not be
relevant for some devices such as CSI-2 receivers. I think it'd make sense
to remove the latter sentence; I can send a patch. The first sentence that
tells the limitations of the function is enough IMO.

-- 
Sakari Ailus
sakari.ai...@linux.intel.com


Re: [PATCH v3 6/6] media: mt9m111: allow to setup pixclk polarity

2018-11-27 Thread Philipp Zabel
Hi Sakari,

On Tue, 2018-11-27 at 15:50 +0200, Sakari Ailus wrote:
> Hi Philipp,
> 
> On Tue, Nov 27, 2018 at 02:39:27PM +0100, Philipp Zabel wrote:
> > Hi Sakari,
> > 
> > On Tue, 2018-11-27 at 15:19 +0200, Sakari Ailus wrote:
> > > Hi Marco,
> > > 
> > > On Tue, Nov 27, 2018 at 11:02:53AM +0100, Marco Felsch wrote:
> > > > From: Enrico Scholz 
> > > > 
> > > > The chip can be configured to output data transitions on the
> > > > rising or falling edge of PIXCLK (Datasheet R58:1[9]), default is on the
> > > > falling edge.
> > > > 
> > > > Parsing the fw-node is made in a subfunction to bundle all (future)
> > > > dt-parsing / fw-parsing stuff.
> > > > 
> > > > Signed-off-by: Enrico Scholz 
> > > > (m.grzesc...@pengutronix.de: Fix inverting clock. INV_PIX_CLOCK bit is 
> > > > set
> > > > per default. Set bit to 0 (enable mask bit without value) to enable
> > > > falling edge sampling.)
> > > > Signed-off-by: Michael Grzeschik 
> > > > (m.fel...@pengutronix.de: use fwnode helpers)
> > > > (m.fel...@pengutronix.de: mv fw parsing into own function)
> > > > (m.fel...@pengutronix.de: adapt commit msg)
> > > > Signed-off-by: Marco Felsch 
> > > 
> > > Applied with the following diff:
> > > 
> > > diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
> > > index 2ef332b9b914..b6011bfddde8 100644
> > > --- a/drivers/media/i2c/mt9m111.c
> > > +++ b/drivers/media/i2c/mt9m111.c
> > > @@ -1172,24 +1172,24 @@ static int mt9m111_video_probe(struct i2c_client 
> > > *client)
> > >  
> > >  static int mt9m111_probe_fw(struct i2c_client *client, struct mt9m111 
> > > *mt9m111)
> > >  {
> > > - struct v4l2_fwnode_endpoint *bus_cfg;
> > > + struct v4l2_fwnode_endpoint bus_cfg = {
> > > + .bus_type = V4L2_MBUS_PARALLEL
> > > + };
> > >   struct fwnode_handle *np;
> > > - int ret = 0;
> > > + int ret;
> > >  
> > >   np = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev), NULL);
> > >   if (!np)
> > >   return -EINVAL;
> > >  
> > > - bus_cfg = v4l2_fwnode_endpoint_alloc_parse(np);
> > > - if (IS_ERR(bus_cfg)) {
> > > - ret = PTR_ERR(bus_cfg);
> > > + ret = v4l2_fwnode_endpoint_alloc_parse(np, &bus_cfg);
> > 
> > Should that be
> > 
> > +   ret = v4l2_fwnode_endpoint_parse(np, &bus_cfg);
> > 
> > intead?
> 
> Could be. I'd expect the driver to need the link frequency at some point
> after which you'd need the variable size properties anyway. But that's not
> the case now.

I don't think the link-frequencies property will be used, this is just a
parallel device. But Marco chose to use _alloc_parse because of what the
v4l2_fwnode_endpoint_parse() documentation says:

/*
 * NOTE: This function does not parse properties the size of which is variable
 * without a low fixed limit. Please use v4l2_fwnode_endpoint_alloc_parse() in  
  
 * new drivers instead.
 */

So maybe we want to use v4l2_fwnode_endpoint_alloc_parse() always. There
is no unnecessary allocation, just a lookup of the non-existing link-
frequencies property.

regards
Philipp


Re: [PATCH v3 6/6] media: mt9m111: allow to setup pixclk polarity

2018-11-27 Thread Sakari Ailus
Hi Philipp,

On Tue, Nov 27, 2018 at 02:39:27PM +0100, Philipp Zabel wrote:
> Hi Sakari,
> 
> On Tue, 2018-11-27 at 15:19 +0200, Sakari Ailus wrote:
> > Hi Marco,
> > 
> > On Tue, Nov 27, 2018 at 11:02:53AM +0100, Marco Felsch wrote:
> > > From: Enrico Scholz 
> > > 
> > > The chip can be configured to output data transitions on the
> > > rising or falling edge of PIXCLK (Datasheet R58:1[9]), default is on the
> > > falling edge.
> > > 
> > > Parsing the fw-node is made in a subfunction to bundle all (future)
> > > dt-parsing / fw-parsing stuff.
> > > 
> > > Signed-off-by: Enrico Scholz 
> > > (m.grzesc...@pengutronix.de: Fix inverting clock. INV_PIX_CLOCK bit is set
> > > per default. Set bit to 0 (enable mask bit without value) to enable
> > > falling edge sampling.)
> > > Signed-off-by: Michael Grzeschik 
> > > (m.fel...@pengutronix.de: use fwnode helpers)
> > > (m.fel...@pengutronix.de: mv fw parsing into own function)
> > > (m.fel...@pengutronix.de: adapt commit msg)
> > > Signed-off-by: Marco Felsch 
> > 
> > Applied with the following diff:
> > 
> > diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
> > index 2ef332b9b914..b6011bfddde8 100644
> > --- a/drivers/media/i2c/mt9m111.c
> > +++ b/drivers/media/i2c/mt9m111.c
> > @@ -1172,24 +1172,24 @@ static int mt9m111_video_probe(struct i2c_client 
> > *client)
> >  
> >  static int mt9m111_probe_fw(struct i2c_client *client, struct mt9m111 
> > *mt9m111)
> >  {
> > -   struct v4l2_fwnode_endpoint *bus_cfg;
> > +   struct v4l2_fwnode_endpoint bus_cfg = {
> > +   .bus_type = V4L2_MBUS_PARALLEL
> > +   };
> > struct fwnode_handle *np;
> > -   int ret = 0;
> > +   int ret;
> >  
> > np = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev), NULL);
> > if (!np)
> > return -EINVAL;
> >  
> > -   bus_cfg = v4l2_fwnode_endpoint_alloc_parse(np);
> > -   if (IS_ERR(bus_cfg)) {
> > -   ret = PTR_ERR(bus_cfg);
> > +   ret = v4l2_fwnode_endpoint_alloc_parse(np, &bus_cfg);
> 
> Should that be
> 
> + ret = v4l2_fwnode_endpoint_parse(np, &bus_cfg);
> 
> intead?

Could be. I'd expect the driver to need the link frequency at some point
after which you'd need the variable size properties anyway. But that's not
the case now.

With the change, the v4l2_fwnode_endpoint_free() becomes redundant. So the
diff on that diff:

diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index b6011bfddde8..d639b9bcf64a 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -1182,15 +1182,13 @@ static int mt9m111_probe_fw(struct i2c_client *client, 
struct mt9m111 *mt9m111)
if (!np)
return -EINVAL;
 
-   ret = v4l2_fwnode_endpoint_alloc_parse(np, &bus_cfg);
+   ret = v4l2_fwnode_endpoint_parse(np, &bus_cfg);
if (ret)
goto out_put_fw;
 
mt9m111->pclk_sample = !!(bus_cfg.bus.parallel.flags &
  V4L2_MBUS_PCLK_SAMPLE_RISING);
 
-   v4l2_fwnode_endpoint_free(&bus_cfg);
-
 out_put_fw:
fwnode_handle_put(np);
return ret;


> 
> > +   if (ret)
> > goto out_put_fw;
> > -   }
> >  
> > -   mt9m111->pclk_sample = !!(bus_cfg->bus.parallel.flags &
> > +   mt9m111->pclk_sample = !!(bus_cfg.bus.parallel.flags &
> >   V4L2_MBUS_PCLK_SAMPLE_RISING);
> >  
> > -   v4l2_fwnode_endpoint_free(bus_cfg);
> > +   v4l2_fwnode_endpoint_free(&bus_cfg);
> >  
> >  out_put_fw:
> > fwnode_handle_put(np);
> > 
> > Please base on current media tree master on the next time. Thanks.
> 
> regards
> Philipp

-- 
Sakari Ailus
sakari.ai...@linux.intel.com


Re: [PATCH v3 6/6] media: mt9m111: allow to setup pixclk polarity

2018-11-27 Thread Marco Felsch
Hi Sakari,

On 18-11-27 15:19, Sakari Ailus wrote:
> Hi Marco,
> 
> On Tue, Nov 27, 2018 at 11:02:53AM +0100, Marco Felsch wrote:
> > From: Enrico Scholz 
> > 
> > The chip can be configured to output data transitions on the
> > rising or falling edge of PIXCLK (Datasheet R58:1[9]), default is on the
> > falling edge.
> > 
> > Parsing the fw-node is made in a subfunction to bundle all (future)
> > dt-parsing / fw-parsing stuff.
> > 
> > Signed-off-by: Enrico Scholz 
> > (m.grzesc...@pengutronix.de: Fix inverting clock. INV_PIX_CLOCK bit is set
> > per default. Set bit to 0 (enable mask bit without value) to enable
> > falling edge sampling.)
> > Signed-off-by: Michael Grzeschik 
> > (m.fel...@pengutronix.de: use fwnode helpers)
> > (m.fel...@pengutronix.de: mv fw parsing into own function)
> > (m.fel...@pengutronix.de: adapt commit msg)
> > Signed-off-by: Marco Felsch 
> 
> Applied with the following diff:
> 
> diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
> index 2ef332b9b914..b6011bfddde8 100644
> --- a/drivers/media/i2c/mt9m111.c
> +++ b/drivers/media/i2c/mt9m111.c
> @@ -1172,24 +1172,24 @@ static int mt9m111_video_probe(struct i2c_client 
> *client)
>  
>  static int mt9m111_probe_fw(struct i2c_client *client, struct mt9m111 
> *mt9m111)
>  {
> - struct v4l2_fwnode_endpoint *bus_cfg;
> + struct v4l2_fwnode_endpoint bus_cfg = {
> + .bus_type = V4L2_MBUS_PARALLEL
> + };
>   struct fwnode_handle *np;
> - int ret = 0;
> + int ret;
>  
>   np = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev), NULL);
>   if (!np)
>   return -EINVAL;
>  
> - bus_cfg = v4l2_fwnode_endpoint_alloc_parse(np);
> - if (IS_ERR(bus_cfg)) {
> - ret = PTR_ERR(bus_cfg);
> + ret = v4l2_fwnode_endpoint_alloc_parse(np, &bus_cfg);
> + if (ret)
>   goto out_put_fw;
> - }
>  
> - mt9m111->pclk_sample = !!(bus_cfg->bus.parallel.flags &
> + mt9m111->pclk_sample = !!(bus_cfg.bus.parallel.flags &
> V4L2_MBUS_PCLK_SAMPLE_RISING);
>  
> - v4l2_fwnode_endpoint_free(bus_cfg);
> + v4l2_fwnode_endpoint_free(&bus_cfg);
>  
>  out_put_fw:
>   fwnode_handle_put(np);
> 
> Please base on current media tree master on the next time. Thanks.

Sorry, thanks for the inline fix :)

Kind regards,
Marco

> -- 
> Kind regards,
> 
> Sakari Ailus
> sakari.ai...@linux.intel.com


Re: [PATCH v3 6/6] media: mt9m111: allow to setup pixclk polarity

2018-11-27 Thread Philipp Zabel
Hi Sakari,

On Tue, 2018-11-27 at 15:19 +0200, Sakari Ailus wrote:
> Hi Marco,
> 
> On Tue, Nov 27, 2018 at 11:02:53AM +0100, Marco Felsch wrote:
> > From: Enrico Scholz 
> > 
> > The chip can be configured to output data transitions on the
> > rising or falling edge of PIXCLK (Datasheet R58:1[9]), default is on the
> > falling edge.
> > 
> > Parsing the fw-node is made in a subfunction to bundle all (future)
> > dt-parsing / fw-parsing stuff.
> > 
> > Signed-off-by: Enrico Scholz 
> > (m.grzesc...@pengutronix.de: Fix inverting clock. INV_PIX_CLOCK bit is set
> > per default. Set bit to 0 (enable mask bit without value) to enable
> > falling edge sampling.)
> > Signed-off-by: Michael Grzeschik 
> > (m.fel...@pengutronix.de: use fwnode helpers)
> > (m.fel...@pengutronix.de: mv fw parsing into own function)
> > (m.fel...@pengutronix.de: adapt commit msg)
> > Signed-off-by: Marco Felsch 
> 
> Applied with the following diff:
> 
> diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
> index 2ef332b9b914..b6011bfddde8 100644
> --- a/drivers/media/i2c/mt9m111.c
> +++ b/drivers/media/i2c/mt9m111.c
> @@ -1172,24 +1172,24 @@ static int mt9m111_video_probe(struct i2c_client 
> *client)
>  
>  static int mt9m111_probe_fw(struct i2c_client *client, struct mt9m111 
> *mt9m111)
>  {
> - struct v4l2_fwnode_endpoint *bus_cfg;
> + struct v4l2_fwnode_endpoint bus_cfg = {
> + .bus_type = V4L2_MBUS_PARALLEL
> + };
>   struct fwnode_handle *np;
> - int ret = 0;
> + int ret;
>  
>   np = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev), NULL);
>   if (!np)
>   return -EINVAL;
>  
> - bus_cfg = v4l2_fwnode_endpoint_alloc_parse(np);
> - if (IS_ERR(bus_cfg)) {
> - ret = PTR_ERR(bus_cfg);
> + ret = v4l2_fwnode_endpoint_alloc_parse(np, &bus_cfg);

Should that be

+   ret = v4l2_fwnode_endpoint_parse(np, &bus_cfg);

intead?

> + if (ret)
>   goto out_put_fw;
> - }
>  
> - mt9m111->pclk_sample = !!(bus_cfg->bus.parallel.flags &
> + mt9m111->pclk_sample = !!(bus_cfg.bus.parallel.flags &
> V4L2_MBUS_PCLK_SAMPLE_RISING);
>  
> - v4l2_fwnode_endpoint_free(bus_cfg);
> + v4l2_fwnode_endpoint_free(&bus_cfg);
>  
>  out_put_fw:
>   fwnode_handle_put(np);
> 
> Please base on current media tree master on the next time. Thanks.

regards
Philipp


Re: [PATCH v3 6/6] media: mt9m111: allow to setup pixclk polarity

2018-11-27 Thread Sakari Ailus
Hi Marco,

On Tue, Nov 27, 2018 at 11:02:53AM +0100, Marco Felsch wrote:
> From: Enrico Scholz 
> 
> The chip can be configured to output data transitions on the
> rising or falling edge of PIXCLK (Datasheet R58:1[9]), default is on the
> falling edge.
> 
> Parsing the fw-node is made in a subfunction to bundle all (future)
> dt-parsing / fw-parsing stuff.
> 
> Signed-off-by: Enrico Scholz 
> (m.grzesc...@pengutronix.de: Fix inverting clock. INV_PIX_CLOCK bit is set
> per default. Set bit to 0 (enable mask bit without value) to enable
> falling edge sampling.)
> Signed-off-by: Michael Grzeschik 
> (m.fel...@pengutronix.de: use fwnode helpers)
> (m.fel...@pengutronix.de: mv fw parsing into own function)
> (m.fel...@pengutronix.de: adapt commit msg)
> Signed-off-by: Marco Felsch 

Applied with the following diff:

diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index 2ef332b9b914..b6011bfddde8 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -1172,24 +1172,24 @@ static int mt9m111_video_probe(struct i2c_client 
*client)
 
 static int mt9m111_probe_fw(struct i2c_client *client, struct mt9m111 *mt9m111)
 {
-   struct v4l2_fwnode_endpoint *bus_cfg;
+   struct v4l2_fwnode_endpoint bus_cfg = {
+   .bus_type = V4L2_MBUS_PARALLEL
+   };
struct fwnode_handle *np;
-   int ret = 0;
+   int ret;
 
np = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev), NULL);
if (!np)
return -EINVAL;
 
-   bus_cfg = v4l2_fwnode_endpoint_alloc_parse(np);
-   if (IS_ERR(bus_cfg)) {
-   ret = PTR_ERR(bus_cfg);
+   ret = v4l2_fwnode_endpoint_alloc_parse(np, &bus_cfg);
+   if (ret)
goto out_put_fw;
-   }
 
-   mt9m111->pclk_sample = !!(bus_cfg->bus.parallel.flags &
+   mt9m111->pclk_sample = !!(bus_cfg.bus.parallel.flags &
  V4L2_MBUS_PCLK_SAMPLE_RISING);
 
-   v4l2_fwnode_endpoint_free(bus_cfg);
+   v4l2_fwnode_endpoint_free(&bus_cfg);
 
 out_put_fw:
fwnode_handle_put(np);

Please base on current media tree master on the next time. Thanks.

-- 
Kind regards,

Sakari Ailus
sakari.ai...@linux.intel.com


[PATCH v3 6/6] media: mt9m111: allow to setup pixclk polarity

2018-11-27 Thread Marco Felsch
From: Enrico Scholz 

The chip can be configured to output data transitions on the
rising or falling edge of PIXCLK (Datasheet R58:1[9]), default is on the
falling edge.

Parsing the fw-node is made in a subfunction to bundle all (future)
dt-parsing / fw-parsing stuff.

Signed-off-by: Enrico Scholz 
(m.grzesc...@pengutronix.de: Fix inverting clock. INV_PIX_CLOCK bit is set
per default. Set bit to 0 (enable mask bit without value) to enable
falling edge sampling.)
Signed-off-by: Michael Grzeschik 
(m.fel...@pengutronix.de: use fwnode helpers)
(m.fel...@pengutronix.de: mv fw parsing into own function)
(m.fel...@pengutronix.de: adapt commit msg)
Signed-off-by: Marco Felsch 

---
Changelog:

v3:
- call mt9m111_probe_fw() before v4l2_clk_get() to avoid error handling

v2:
- make use of fwnode_*() to drop OF dependency and ifdef's
- mt9m111_g_mbus_config: fix pclk_sample logic which I made due the
  conversion from Enrico's patch.
---
 drivers/media/i2c/mt9m111.c | 46 -
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index f97fd32181ed..2ef332b9b914 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -22,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /*
  * MT9M111, MT9M112 and MT9M131:
@@ -242,6 +244,8 @@ struct mt9m111 {
const struct mt9m111_datafmt *fmt;
int lastpage;   /* PageMap cache value */
bool is_streaming;
+   /* user point of view - 0: falling 1: rising edge */
+   unsigned int pclk_sample:1;
 #ifdef CONFIG_MEDIA_CONTROLLER
struct media_pad pad;
 #endif
@@ -594,6 +598,10 @@ static int mt9m111_set_pixfmt(struct mt9m111 *mt9m111,
return -EINVAL;
}
 
+   /* receiver samples on falling edge, chip-hw default is rising */
+   if (mt9m111->pclk_sample == 0)
+   mask_outfmt2 |= MT9M111_OUTFMT_INV_PIX_CLOCK;
+
ret = mt9m111_reg_mask(client, context_a.output_fmt_ctrl2,
   data_outfmt2, mask_outfmt2);
if (!ret)
@@ -1084,9 +1092,15 @@ static int mt9m111_s_stream(struct v4l2_subdev *sd, int 
enable)
 static int mt9m111_g_mbus_config(struct v4l2_subdev *sd,
struct v4l2_mbus_config *cfg)
 {
-   cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
+   struct mt9m111 *mt9m111 = container_of(sd, struct mt9m111, subdev);
+
+   cfg->flags = V4L2_MBUS_MASTER |
V4L2_MBUS_HSYNC_ACTIVE_HIGH | V4L2_MBUS_VSYNC_ACTIVE_HIGH |
V4L2_MBUS_DATA_ACTIVE_HIGH;
+
+   cfg->flags |= mt9m111->pclk_sample ? V4L2_MBUS_PCLK_SAMPLE_RISING :
+   V4L2_MBUS_PCLK_SAMPLE_FALLING;
+
cfg->type = V4L2_MBUS_PARALLEL;
 
return 0;
@@ -1156,6 +1170,32 @@ static int mt9m111_video_probe(struct i2c_client *client)
return ret;
 }
 
+static int mt9m111_probe_fw(struct i2c_client *client, struct mt9m111 *mt9m111)
+{
+   struct v4l2_fwnode_endpoint *bus_cfg;
+   struct fwnode_handle *np;
+   int ret = 0;
+
+   np = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev), NULL);
+   if (!np)
+   return -EINVAL;
+
+   bus_cfg = v4l2_fwnode_endpoint_alloc_parse(np);
+   if (IS_ERR(bus_cfg)) {
+   ret = PTR_ERR(bus_cfg);
+   goto out_put_fw;
+   }
+
+   mt9m111->pclk_sample = !!(bus_cfg->bus.parallel.flags &
+ V4L2_MBUS_PCLK_SAMPLE_RISING);
+
+   v4l2_fwnode_endpoint_free(bus_cfg);
+
+out_put_fw:
+   fwnode_handle_put(np);
+   return ret;
+}
+
 static int mt9m111_probe(struct i2c_client *client,
 const struct i2c_device_id *did)
 {
@@ -1173,6 +1213,10 @@ static int mt9m111_probe(struct i2c_client *client,
if (!mt9m111)
return -ENOMEM;
 
+   ret = mt9m111_probe_fw(client, mt9m111);
+   if (ret)
+   return ret;
+
mt9m111->clk = v4l2_clk_get(&client->dev, "mclk");
if (IS_ERR(mt9m111->clk))
return PTR_ERR(mt9m111->clk);
-- 
2.19.1