* [bug report] drm/bridge: Add ti-tfp410 DVI transmitter driver
@ 2016-12-06 13:28 Dan Carpenter
2016-12-07 14:20 ` Jyri Sarha
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2016-12-06 13:28 UTC (permalink / raw)
To: jsarha; +Cc: dri-devel
Hello Jyri Sarha,
The patch dc55ac3b52e6: "drm/bridge: Add ti-tfp410 DVI transmitter
driver" from Oct 31, 2016, leads to the following static checker
warning:
drivers/gpu/drm/bridge/ti-tfp410.c:141 tfp410_get_connector_ddc()
warn: missing error code here? 'of_graph_get_remote_port_parent()' failed.
drivers/gpu/drm/bridge/ti-tfp410.c
128 static int tfp410_get_connector_ddc(struct tfp410 *dvi)
129 {
130 struct device_node *ep = NULL, *connector_node = NULL;
131 struct device_node *ddc_phandle = NULL;
132 int ret = 0;
133
134 /* port@1 is the connector node */
135 ep = of_graph_get_endpoint_by_regs(dvi->dev->of_node, 1, -1);
136 if (!ep)
137 goto fail;
^^^^^^^^^
138
139 connector_node = of_graph_get_remote_port_parent(ep);
140 if (!connector_node)
141 goto fail;
^^^^^^^^^
142
143 ddc_phandle = of_parse_phandle(connector_node, "ddc-i2c-bus", 0);
144 if (!ddc_phandle)
145 goto fail;
^^^^^^^^^
All these goto fails feel like we should be returning an error code.
Otherwise we could change the name to "goto success;"?
146
147 dvi->ddc = of_get_i2c_adapter_by_node(ddc_phandle);
148 if (dvi->ddc)
149 dev_info(dvi->dev, "Connector's ddc i2c bus found\n");
150 else
151 ret = -EPROBE_DEFER;
152
153 fail:
154 of_node_put(ep);
155 of_node_put(connector_node);
156 of_node_put(ddc_phandle);
157 return ret;
158 }
regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] drm/bridge: Add ti-tfp410 DVI transmitter driver
2016-12-06 13:28 [bug report] drm/bridge: Add ti-tfp410 DVI transmitter driver Dan Carpenter
@ 2016-12-07 14:20 ` Jyri Sarha
2016-12-07 14:46 ` Daniel Vetter
0 siblings, 1 reply; 5+ messages in thread
From: Jyri Sarha @ 2016-12-07 14:20 UTC (permalink / raw)
To: Dan Carpenter; +Cc: dri-devel
On 12/06/16 15:28, Dan Carpenter wrote:
> Hello Jyri Sarha,
>
> The patch dc55ac3b52e6: "drm/bridge: Add ti-tfp410 DVI transmitter
> driver" from Oct 31, 2016, leads to the following static checker
> warning:
>
> drivers/gpu/drm/bridge/ti-tfp410.c:141 tfp410_get_connector_ddc()
> warn: missing error code here? 'of_graph_get_remote_port_parent()' failed.
>
> drivers/gpu/drm/bridge/ti-tfp410.c
> 128 static int tfp410_get_connector_ddc(struct tfp410 *dvi)
> 129 {
> 130 struct device_node *ep = NULL, *connector_node = NULL;
> 131 struct device_node *ddc_phandle = NULL;
> 132 int ret = 0;
> 133
> 134 /* port@1 is the connector node */
> 135 ep = of_graph_get_endpoint_by_regs(dvi->dev->of_node, 1, -1);
> 136 if (!ep)
> 137 goto fail;
> ^^^^^^^^^
> 138
> 139 connector_node = of_graph_get_remote_port_parent(ep);
> 140 if (!connector_node)
> 141 goto fail;
> ^^^^^^^^^
>
>
> 142
> 143 ddc_phandle = of_parse_phandle(connector_node, "ddc-i2c-bus", 0);
> 144 if (!ddc_phandle)
> 145 goto fail;
> ^^^^^^^^^
>
> All these goto fails feel like we should be returning an error code.
> Otherwise we could change the name to "goto success;"?
>
The label text could be seen misleading, but code runs as I intended it.
If we can not extract the ddc i2c controller from the binding, we simply
continue without it. The only exception is if we find the phandle but
not the i2c controller, yet.
This implementation is a hack in the first place. The code will be
removed when DRM has a proper generic connector support.
Best regards,
Jyri
> 146
> 147 dvi->ddc = of_get_i2c_adapter_by_node(ddc_phandle);
> 148 if (dvi->ddc)
> 149 dev_info(dvi->dev, "Connector's ddc i2c bus found\n");
> 150 else
> 151 ret = -EPROBE_DEFER;
> 152
> 153 fail:
> 154 of_node_put(ep);
> 155 of_node_put(connector_node);
> 156 of_node_put(ddc_phandle);
> 157 return ret;
> 158 }
>
> regards,
> dan carpenter
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] drm/bridge: Add ti-tfp410 DVI transmitter driver
2016-12-07 14:20 ` Jyri Sarha
@ 2016-12-07 14:46 ` Daniel Vetter
2016-12-07 20:28 ` Jyri Sarha
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Vetter @ 2016-12-07 14:46 UTC (permalink / raw)
To: Jyri Sarha; +Cc: dri-devel, Dan Carpenter
On Wed, Dec 07, 2016 at 04:20:34PM +0200, Jyri Sarha wrote:
> On 12/06/16 15:28, Dan Carpenter wrote:
> > Hello Jyri Sarha,
> >
> > The patch dc55ac3b52e6: "drm/bridge: Add ti-tfp410 DVI transmitter
> > driver" from Oct 31, 2016, leads to the following static checker
> > warning:
> >
> > drivers/gpu/drm/bridge/ti-tfp410.c:141 tfp410_get_connector_ddc()
> > warn: missing error code here? 'of_graph_get_remote_port_parent()' failed.
> >
> > drivers/gpu/drm/bridge/ti-tfp410.c
> > 128 static int tfp410_get_connector_ddc(struct tfp410 *dvi)
> > 129 {
> > 130 struct device_node *ep = NULL, *connector_node = NULL;
> > 131 struct device_node *ddc_phandle = NULL;
> > 132 int ret = 0;
> > 133
> > 134 /* port@1 is the connector node */
> > 135 ep = of_graph_get_endpoint_by_regs(dvi->dev->of_node, 1, -1);
> > 136 if (!ep)
> > 137 goto fail;
> > ^^^^^^^^^
> > 138
> > 139 connector_node = of_graph_get_remote_port_parent(ep);
> > 140 if (!connector_node)
> > 141 goto fail;
> > ^^^^^^^^^
> >
> >
> > 142
> > 143 ddc_phandle = of_parse_phandle(connector_node, "ddc-i2c-bus", 0);
> > 144 if (!ddc_phandle)
> > 145 goto fail;
> > ^^^^^^^^^
> >
> > All these goto fails feel like we should be returning an error code.
> > Otherwise we could change the name to "goto success;"?
> >
>
> The label text could be seen misleading, but code runs as I intended it.
> If we can not extract the ddc i2c controller from the binding, we simply
> continue without it. The only exception is if we find the phandle but
> not the i2c controller, yet.
>
> This implementation is a hack in the first place. The code will be
> removed when DRM has a proper generic connector support.
Hm, what's this generic drm connector thing you talk about? What's it for?
Asking since I haven't seen that yet nor heard in a discussion ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] drm/bridge: Add ti-tfp410 DVI transmitter driver
2016-12-07 14:46 ` Daniel Vetter
@ 2016-12-07 20:28 ` Jyri Sarha
2016-12-07 22:29 ` Laurent Pinchart
0 siblings, 1 reply; 5+ messages in thread
From: Jyri Sarha @ 2016-12-07 20:28 UTC (permalink / raw)
To: Daniel Vetter; +Cc: dri-devel, Dan Carpenter, Laurent Pinchart
On 12/07/16 16:46, Daniel Vetter wrote:
>> > This implementation is a hack in the first place. The code will be
>> > removed when DRM has a proper generic connector support.
> Hm, what's this generic drm connector thing you talk about? What's it for?
> Asking since I haven't seen that yet nor heard in a discussion ...
After looking up this comment from Laurent:
On 11/10/16 14:15, Laurent Pinchart wrote [1]:
>> > The implementation side is not so critical, because it more easily
>> > changed, but should I create an independent generic platform-device
>> > driver for such DVI/HDMI connector or just implement the connector side
>> > within tfp410 driver?
> Longer term I'd like to go for connector drivers, but it might take a
bit of
> infrastructure work. If you can give it a try it would be great !
Otherwise
> I'm fine with handling that in the tfp410 driver for now.
>
... I think I miss understood it. He was probably referring to the
infrastructure needed in the tfp410 driver. I'll try to get that done
for 4.11 or 4.12.
Best regards,
Jyri
[1]
https://lists.freedesktop.org/archives/dri-devel/2016-November/123337.html
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] drm/bridge: Add ti-tfp410 DVI transmitter driver
2016-12-07 20:28 ` Jyri Sarha
@ 2016-12-07 22:29 ` Laurent Pinchart
0 siblings, 0 replies; 5+ messages in thread
From: Laurent Pinchart @ 2016-12-07 22:29 UTC (permalink / raw)
To: Jyri Sarha; +Cc: dri-devel, Dan Carpenter
Hi Jyri,
On Wednesday 07 Dec 2016 22:28:56 Jyri Sarha wrote:
> On 12/07/16 16:46, Daniel Vetter wrote:
> >>> This implementation is a hack in the first place. The code will be
> >>> removed when DRM has a proper generic connector support.
> >
> > Hm, what's this generic drm connector thing you talk about? What's it for?
> > Asking since I haven't seen that yet nor heard in a discussion ...
>
> After looking up this comment from Laurent:
>
> On 11/10/16 14:15, Laurent Pinchart wrote [1]:
> >>> The implementation side is not so critical, because it more easily
> >>> changed, but should I create an independent generic platform-device
> >>> driver for such DVI/HDMI connector or just implement the connector side
> >>> within tfp410 driver?
> >
> > Longer term I'd like to go for connector drivers, but it might take a
> > bit of infrastructure work. If you can give it a try it would be great !
> > Otherwise I'm fine with handling that in the tfp410 driver for now.
>
> ... I think I miss understood it. He was probably referring to the
> infrastructure needed in the tfp410 driver. I'll try to get that done
> for 4.11 or 4.12.
I was referring to drivers for connectors, binding to the connectors DT nodes.
We shouldn't duplicate handling of connector-related features (ddc-i2c-bus
being the most important one) in every bridge driver, and we need to pave the
way for bridges chained with other bridges instead of being connected directly
to a connector.
> [1]
> https://lists.freedesktop.org/archives/dri-devel/2016-November/123337.html
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-12-07 22:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-06 13:28 [bug report] drm/bridge: Add ti-tfp410 DVI transmitter driver Dan Carpenter
2016-12-07 14:20 ` Jyri Sarha
2016-12-07 14:46 ` Daniel Vetter
2016-12-07 20:28 ` Jyri Sarha
2016-12-07 22:29 ` Laurent Pinchart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.