| Message ID | 20181127100253.30845-7-m.felsch@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: mt9m111 features | expand |
Hi Marco, On Tue, Nov 27, 2018 at 11:02:53AM +0100, Marco Felsch wrote: > From: Enrico Scholz <enrico.scholz@sigma-chemnitz.de> > > 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 <enrico.scholz@sigma-chemnitz.de> > (m.grzeschik@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.grzeschik@pengutronix.de> > (m.felsch@pengutronix.de: use fwnode helpers) > (m.felsch@pengutronix.de: mv fw parsing into own function) > (m.felsch@pengutronix.de: adapt commit msg) > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> 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.
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 <enrico.scholz@sigma-chemnitz.de> > > > > 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 <enrico.scholz@sigma-chemnitz.de> > > (m.grzeschik@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.grzeschik@pengutronix.de> > > (m.felsch@pengutronix.de: use fwnode helpers) > > (m.felsch@pengutronix.de: mv fw parsing into own function) > > (m.felsch@pengutronix.de: adapt commit msg) > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > 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
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 <enrico.scholz@sigma-chemnitz.de> > > > > 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 <enrico.scholz@sigma-chemnitz.de> > > (m.grzeschik@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.grzeschik@pengutronix.de> > > (m.felsch@pengutronix.de: use fwnode helpers) > > (m.felsch@pengutronix.de: mv fw parsing into own function) > > (m.felsch@pengutronix.de: adapt commit msg) > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > 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.ailus@linux.intel.com
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 <enrico.scholz@sigma-chemnitz.de> > > > > > > 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 <enrico.scholz@sigma-chemnitz.de> > > > (m.grzeschik@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.grzeschik@pengutronix.de> > > > (m.felsch@pengutronix.de: use fwnode helpers) > > > (m.felsch@pengutronix.de: mv fw parsing into own function) > > > (m.felsch@pengutronix.de: adapt commit msg) > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > > > 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
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 <enrico.scholz@sigma-chemnitz.de> > > > > > > > > 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 <enrico.scholz@sigma-chemnitz.de> > > > > (m.grzeschik@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.grzeschik@pengutronix.de> > > > > (m.felsch@pengutronix.de: use fwnode helpers) > > > > (m.felsch@pengutronix.de: mv fw parsing into own function) > > > > (m.felsch@pengutronix.de: adapt commit msg) > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > > > > > 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
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 <enrico.scholz@sigma-chemnitz.de> > > > > > > > > > > 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 <enrico.scholz@sigma-chemnitz.de> > > > > > (m.grzeschik@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.grzeschik@pengutronix.de> > > > > > (m.felsch@pengutronix.de: use fwnode helpers) > > > > > (m.felsch@pengutronix.de: mv fw parsing into own function) > > > > > (m.felsch@pengutronix.de: adapt commit msg) > > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > > > > > > > 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.
On Tue, Nov 27, 2018 at 11:02:53AM +0100, Marco Felsch wrote: > From: Enrico Scholz <enrico.scholz@sigma-chemnitz.de> > > 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 <enrico.scholz@sigma-chemnitz.de> > (m.grzeschik@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.grzeschik@pengutronix.de> > (m.felsch@pengutronix.de: use fwnode helpers) > (m.felsch@pengutronix.de: mv fw parsing into own function) > (m.felsch@pengutronix.de: adapt commit msg) > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> 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
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 <enrico.scholz@sigma-chemnitz.de> > > > > 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 <enrico.scholz@sigma-chemnitz.de> > > (m.grzeschik@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.grzeschik@pengutronix.de> > > (m.felsch@pengutronix.de: use fwnode helpers) > > (m.felsch@pengutronix.de: mv fw parsing into own function) > > (m.felsch@pengutronix.de: adapt commit msg) > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > 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.ailus@linux.intel.com >
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 <enrico.scholz@sigma-chemnitz.de> > > > > > > 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 <enrico.scholz@sigma-chemnitz.de> > > > (m.grzeschik@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.grzeschik@pengutronix.de> > > > (m.felsch@pengutronix.de: use fwnode helpers) > > > (m.felsch@pengutronix.de: mv fw parsing into own function) > > > (m.felsch@pengutronix.de: adapt commit msg) > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > > > 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...
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 <linux/delay.h> #include <linux/v4l2-mediabus.h> #include <linux/module.h> +#include <linux/property.h> #include <media/v4l2-async.h> #include <media/v4l2-clk.h> @@ -22,6 +23,7 @@ #include <media/v4l2-ctrls.h> #include <media/v4l2-device.h> #include <media/v4l2-event.h> +#include <media/v4l2-fwnode.h> /* * 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);