Linux-Renesas-SoC Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v9.1 0/3] MAX9286 fixups
@ 2020-05-14 14:00 Kieran Bingham
  2020-05-14 14:00 ` [PATCH v9.1 1/3] fixes! [max9286]: Use the same default mbus_fmt everywhere Kieran Bingham
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Kieran Bingham @ 2020-05-14 14:00 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: Kieran Bingham, Laurent Pinchart, Jacopo Mondi,
	Niklas Söderlund, Hyun Kwon, Manivannan Sadhasivam,
	Kieran Bingham

Following the review comments on v9, max9286, here are the fixes I plan
to apply to generate v10.

Kieran Bingham (3):
  fixes! [max9286]: Use the same default mbus_fmt everywhere
  fixes! [max9286]: Don't provide GPIO names
  fixes! [max9286]: Fix dev->of_node refcnting

 drivers/media/i2c/max9286.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v9.1 1/3] fixes! [max9286]: Use the same default mbus_fmt everywhere
  2020-05-14 14:00 [PATCH v9.1 0/3] MAX9286 fixups Kieran Bingham
@ 2020-05-14 14:00 ` Kieran Bingham
  2020-05-14 14:00 ` [PATCH v9 2/3] fixes! [max9286]: Don't provide GPIO names Kieran Bingham
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Kieran Bingham @ 2020-05-14 14:00 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: Kieran Bingham, Laurent Pinchart, Jacopo Mondi,
	Niklas Söderlund, Hyun Kwon, Manivannan Sadhasivam,
	Kieran Bingham

The default code used in max9286_set_fmt() differs from other defaults.
Correct it.

Reported-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/media/i2c/max9286.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index 481d65f2b51d..c8ca1245df4b 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -722,7 +722,7 @@ static int max9286_set_fmt(struct v4l2_subdev *sd,
 	case MEDIA_BUS_FMT_YVYU8_2X8:
 		break;
 	default:
-		format->format.code = MEDIA_BUS_FMT_YUYV8_2X8;
+		format->format.code = MEDIA_BUS_FMT_UYVY8_2X8;
 		break;
 	}
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v9 2/3] fixes! [max9286]: Don't provide GPIO names
  2020-05-14 14:00 [PATCH v9.1 0/3] MAX9286 fixups Kieran Bingham
  2020-05-14 14:00 ` [PATCH v9.1 1/3] fixes! [max9286]: Use the same default mbus_fmt everywhere Kieran Bingham
@ 2020-05-14 14:00 ` Kieran Bingham
  2020-05-14 14:00 ` [PATCH v9 3/3] fixes! [max9286]: Fix dev->of_node refcnting Kieran Bingham
  2020-05-16 14:25 ` [PATCH v9.1 0/3] MAX9286 fixups Jacopo Mondi
  3 siblings, 0 replies; 5+ messages in thread
From: Kieran Bingham @ 2020-05-14 14:00 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: Kieran Bingham, Laurent Pinchart, Jacopo Mondi,
	Niklas Söderlund, Hyun Kwon, Manivannan Sadhasivam,
	Kieran Bingham

The GPIO line names are fairly unhelpful, other than describing
them as out lines only ... but otherwise ...

Having multiple gpio devices with the same names reports conflicts.
so lets just remove them.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/media/i2c/max9286.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index c8ca1245df4b..66201dc4b7f7 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -1019,11 +1019,6 @@ static int max9286_register_gpio(struct max9286_priv *priv)
 	struct gpio_chip *gpio = &priv->gpio;
 	int ret;
 
-	static const char * const names[] = {
-		"GPIO0OUT",
-		"GPIO1OUT",
-	};
-
 	/* Configure the GPIO */
 	gpio->label = dev_name(dev);
 	gpio->parent = dev;
@@ -1034,7 +1029,6 @@ static int max9286_register_gpio(struct max9286_priv *priv)
 	gpio->set = max9286_gpio_set;
 	gpio->get = max9286_gpio_get;
 	gpio->can_sleep = true;
-	gpio->names = names;
 
 	/* GPIO values default to high */
 	priv->gpio_state = BIT(0) | BIT(1);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v9 3/3] fixes! [max9286]: Fix dev->of_node refcnting
  2020-05-14 14:00 [PATCH v9.1 0/3] MAX9286 fixups Kieran Bingham
  2020-05-14 14:00 ` [PATCH v9.1 1/3] fixes! [max9286]: Use the same default mbus_fmt everywhere Kieran Bingham
  2020-05-14 14:00 ` [PATCH v9 2/3] fixes! [max9286]: Don't provide GPIO names Kieran Bingham
@ 2020-05-14 14:00 ` Kieran Bingham
  2020-05-16 14:25 ` [PATCH v9.1 0/3] MAX9286 fixups Jacopo Mondi
  3 siblings, 0 replies; 5+ messages in thread
From: Kieran Bingham @ 2020-05-14 14:00 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: Kieran Bingham, Laurent Pinchart, Jacopo Mondi,
	Niklas Söderlund, Hyun Kwon, Manivannan Sadhasivam,
	Kieran Bingham

With a little help from an of_ref_read():

+static int of_ref_read(struct device_node *node)
+{
+       if (node)
+               return kref_read(&node->kobj.kref);
+
+       return 0;
+}

I've validated the refcount of the node now stays consitent.
Particularly between entry (A) and exit (B) of the parse_dt function:

[    2.305784] max9286 4-004c: A: node refcnt is 6
[    2.310401] max9286 4-004c: node refcnt is 6
[    2.314729] max9286 4-004c: 1335: node refcnt is 6
[    2.319587] max9286 4-004c: 1356: node refcnt is 6
[    2.324432] max9286 4-004c: 1364: (in for_each) node refcnt is 6
[    2.330503] max9286 4-004c: 1364: (in for_each) node refcnt is 6
[    2.336575] max9286 4-004c: 1364: (in for_each) node refcnt is 6
[    2.342656] max9286 4-004c: 1364: (in for_each) node refcnt is 6
[    2.348724] max9286 4-004c: 1364: (in for_each) node refcnt is 6
[    2.354808] max9286 4-004c: 1437: node refcnt is 6
[    2.359644] max9286 4-004c: B: node refcnt is 6

I've added a comment to explain the extra of_node_get() but the exercise
identified that the driver was incorrectly calling of_node_put() on the
same node. Those have been removed.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/media/i2c/max9286.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index 66201dc4b7f7..590f384161a5 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -1108,11 +1108,11 @@ static int max9286_parse_dt(struct max9286_priv *priv)
 	struct device_node *node = NULL;
 	unsigned int i2c_mux_mask = 0;
 
+	/* Balance the of_node_put() performed by of_find_node_by_name(). */
 	of_node_get(dev->of_node);
 	i2c_mux = of_find_node_by_name(dev->of_node, "i2c-mux");
 	if (!i2c_mux) {
 		dev_err(dev, "Failed to find i2c-mux node\n");
-		of_node_put(dev->of_node);
 		return -EINVAL;
 	}
 
@@ -1160,7 +1160,6 @@ static int max9286_parse_dt(struct max9286_priv *priv)
 					of_fwnode_handle(node), &vep);
 			if (ret) {
 				of_node_put(node);
-				of_node_put(dev->of_node);
 				return ret;
 			}
 
@@ -1170,7 +1169,6 @@ static int max9286_parse_dt(struct max9286_priv *priv)
 					vep.bus_type);
 				v4l2_fwnode_endpoint_free(&vep);
 				of_node_put(node);
-				of_node_put(dev->of_node);
 				return -EINVAL;
 			}
 
@@ -1208,7 +1206,6 @@ static int max9286_parse_dt(struct max9286_priv *priv)
 		priv->nsources++;
 	}
 	of_node_put(node);
-	of_node_put(dev->of_node);
 
 	priv->route_mask = priv->source_mask;
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v9.1 0/3] MAX9286 fixups
  2020-05-14 14:00 [PATCH v9.1 0/3] MAX9286 fixups Kieran Bingham
                   ` (2 preceding siblings ...)
  2020-05-14 14:00 ` [PATCH v9 3/3] fixes! [max9286]: Fix dev->of_node refcnting Kieran Bingham
@ 2020-05-16 14:25 ` Jacopo Mondi
  3 siblings, 0 replies; 5+ messages in thread
From: Jacopo Mondi @ 2020-05-16 14:25 UTC (permalink / raw)
  To: Kieran Bingham
  Cc: linux-renesas-soc, Kieran Bingham, Laurent Pinchart,
	Niklas Söderlund, Hyun Kwon, Manivannan Sadhasivam

Hi Kieran,

On Thu, May 14, 2020 at 03:00:13PM +0100, Kieran Bingham wrote:
> Following the review comments on v9, max9286, here are the fixes I plan
> to apply to generate v10.

Please squash in v10

Acked-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j
>
> Kieran Bingham (3):
>   fixes! [max9286]: Use the same default mbus_fmt everywhere
>   fixes! [max9286]: Don't provide GPIO names
>   fixes! [max9286]: Fix dev->of_node refcnting
>
>  drivers/media/i2c/max9286.c | 13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)
>
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-14 14:00 [PATCH v9.1 0/3] MAX9286 fixups Kieran Bingham
2020-05-14 14:00 ` [PATCH v9.1 1/3] fixes! [max9286]: Use the same default mbus_fmt everywhere Kieran Bingham
2020-05-14 14:00 ` [PATCH v9 2/3] fixes! [max9286]: Don't provide GPIO names Kieran Bingham
2020-05-14 14:00 ` [PATCH v9 3/3] fixes! [max9286]: Fix dev->of_node refcnting Kieran Bingham
2020-05-16 14:25 ` [PATCH v9.1 0/3] MAX9286 fixups Jacopo Mondi

Linux-Renesas-SoC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-renesas-soc/0 linux-renesas-soc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-renesas-soc linux-renesas-soc/ https://lore.kernel.org/linux-renesas-soc \
		linux-renesas-soc@vger.kernel.org
	public-inbox-index linux-renesas-soc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-renesas-soc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git