2019-10-14 23:34:29

by Dmitry Torokhov

[permalink] [raw]
Subject: [PATCH] drm/bridge: ti-tfp410: switch to using fwnode_gpiod_get_index()

Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use
the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), but
works with arbitrary firmware node.

Reviewed-by: Laurent Pinchart <[email protected]>
Signed-off-by: Dmitry Torokhov <[email protected]>
---

Andrzej, Neil,

This depends on the new code that can be bound in
ib-fwnode-gpiod-get-index immutable branch of Linus' Walleij tree:

git pull git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git ib-fwnode-gpiod-get-index

I hope that it would be possible to pull in this immutable branch and
not wait until after 5.5 merge window, or, alternatively, merge through
Linus Walleij's tree.

Thanks!

drivers/gpu/drm/bridge/ti-tfp410.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
index aa3198dc9903..6f6d6d1e60ae 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -285,8 +285,8 @@ static int tfp410_get_connector_properties(struct tfp410 *dvi)
else
dvi->connector_type = DRM_MODE_CONNECTOR_DVID;

- dvi->hpd = fwnode_get_named_gpiod(&connector_node->fwnode,
- "hpd-gpios", 0, GPIOD_IN, "hpd");
+ dvi->hpd = fwnode_gpiod_get_index(&connector_node->fwnode,
+ "hpd", 0, GPIOD_IN, "hpd");
if (IS_ERR(dvi->hpd)) {
ret = PTR_ERR(dvi->hpd);
dvi->hpd = NULL;
--
2.23.0.700.g56cf767bdb-goog


--
Dmitry


2019-10-15 00:44:44

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] drm/bridge: ti-tfp410: switch to using fwnode_gpiod_get_index()

Hi Dmitry,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.4-rc3 next-20191014]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Dmitry-Torokhov/drm-bridge-ti-tfp410-switch-to-using-fwnode_gpiod_get_index/20191015-024641
config: mips-allmodconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=mips

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All error/warnings (new ones prefixed by >>):

drivers/gpu//drm/bridge/ti-tfp410.c: In function 'tfp410_get_connector_properties':
>> drivers/gpu//drm/bridge/ti-tfp410.c:287:13: error: implicit declaration of function 'fwnode_gpiod_get_index'; did you mean 'devm_gpiod_get_index'? [-Werror=implicit-function-declaration]
dvi->hpd = fwnode_gpiod_get_index(&connector_node->fwnode,
^~~~~~~~~~~~~~~~~~~~~~
devm_gpiod_get_index
>> drivers/gpu//drm/bridge/ti-tfp410.c:287:11: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
dvi->hpd = fwnode_gpiod_get_index(&connector_node->fwnode,
^
cc1: some warnings being treated as errors

vim +287 drivers/gpu//drm/bridge/ti-tfp410.c

271
272 static int tfp410_get_connector_properties(struct tfp410 *dvi)
273 {
274 struct device_node *connector_node, *ddc_phandle;
275 int ret = 0;
276
277 /* port@1 is the connector node */
278 connector_node = of_graph_get_remote_node(dvi->dev->of_node, 1, -1);
279 if (!connector_node)
280 return -ENODEV;
281
282 if (of_device_is_compatible(connector_node, "hdmi-connector"))
283 dvi->connector_type = DRM_MODE_CONNECTOR_HDMIA;
284 else
285 dvi->connector_type = DRM_MODE_CONNECTOR_DVID;
286
> 287 dvi->hpd = fwnode_gpiod_get_index(&connector_node->fwnode,
288 "hpd", 0, GPIOD_IN, "hpd");
289 if (IS_ERR(dvi->hpd)) {
290 ret = PTR_ERR(dvi->hpd);
291 dvi->hpd = NULL;
292 if (ret == -ENOENT)
293 ret = 0;
294 else
295 goto fail;
296 }
297
298 ddc_phandle = of_parse_phandle(connector_node, "ddc-i2c-bus", 0);
299 if (!ddc_phandle)
300 goto fail;
301
302 dvi->ddc = of_get_i2c_adapter_by_node(ddc_phandle);
303 if (dvi->ddc)
304 dev_info(dvi->dev, "Connector's ddc i2c bus found\n");
305 else
306 ret = -EPROBE_DEFER;
307
308 of_node_put(ddc_phandle);
309
310 fail:
311 of_node_put(connector_node);
312 return ret;
313 }
314

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (3.21 kB)
.config.gz (60.61 kB)
Download all attachments

2019-10-15 05:39:58

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] drm/bridge: ti-tfp410: switch to using fwnode_gpiod_get_index()

Hi Dmitry,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[cannot apply to v5.4-rc3 next-20191014]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Dmitry-Torokhov/drm-bridge-ti-tfp410-switch-to-using-fwnode_gpiod_get_index/20191015-024641
config: nds32-allmodconfig (attached as .config)
compiler: nds32le-linux-gcc (GCC) 8.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=nds32

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All warnings (new ones prefixed by >>):

drivers/gpu/drm/bridge/ti-tfp410.c: In function 'tfp410_get_connector_properties':
drivers/gpu/drm/bridge/ti-tfp410.c:287:13: error: implicit declaration of function 'fwnode_gpiod_get_index'; did you mean 'devm_gpiod_get_index'? [-Werror=implicit-function-declaration]
dvi->hpd = fwnode_gpiod_get_index(&connector_node->fwnode,
^~~~~~~~~~~~~~~~~~~~~~
devm_gpiod_get_index
>> drivers/gpu/drm/bridge/ti-tfp410.c:287:11: warning: assignment to 'struct gpio_desc *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
dvi->hpd = fwnode_gpiod_get_index(&connector_node->fwnode,
^
cc1: some warnings being treated as errors

vim +287 drivers/gpu/drm/bridge/ti-tfp410.c

271
272 static int tfp410_get_connector_properties(struct tfp410 *dvi)
273 {
274 struct device_node *connector_node, *ddc_phandle;
275 int ret = 0;
276
277 /* port@1 is the connector node */
278 connector_node = of_graph_get_remote_node(dvi->dev->of_node, 1, -1);
279 if (!connector_node)
280 return -ENODEV;
281
282 if (of_device_is_compatible(connector_node, "hdmi-connector"))
283 dvi->connector_type = DRM_MODE_CONNECTOR_HDMIA;
284 else
285 dvi->connector_type = DRM_MODE_CONNECTOR_DVID;
286
> 287 dvi->hpd = fwnode_gpiod_get_index(&connector_node->fwnode,
288 "hpd", 0, GPIOD_IN, "hpd");
289 if (IS_ERR(dvi->hpd)) {
290 ret = PTR_ERR(dvi->hpd);
291 dvi->hpd = NULL;
292 if (ret == -ENOENT)
293 ret = 0;
294 else
295 goto fail;
296 }
297
298 ddc_phandle = of_parse_phandle(connector_node, "ddc-i2c-bus", 0);
299 if (!ddc_phandle)
300 goto fail;
301
302 dvi->ddc = of_get_i2c_adapter_by_node(ddc_phandle);
303 if (dvi->ddc)
304 dev_info(dvi->dev, "Connector's ddc i2c bus found\n");
305 else
306 ret = -EPROBE_DEFER;
307
308 of_node_put(ddc_phandle);
309
310 fail:
311 of_node_put(connector_node);
312 return ret;
313 }
314

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (3.24 kB)
.config.gz (51.75 kB)
Download all attachments

2019-11-05 00:45:19

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] drm/bridge: ti-tfp410: switch to using fwnode_gpiod_get_index()

On Mon, Oct 14, 2019 at 11:43:20AM -0700, Dmitry Torokhov wrote:
> Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use
> the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), but
> works with arbitrary firmware node.
>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Signed-off-by: Dmitry Torokhov <[email protected]>
> ---
>
> Andrzej, Neil,
>
> This depends on the new code that can be bound in
> ib-fwnode-gpiod-get-index immutable branch of Linus' Walleij tree:
>
> git pull git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git ib-fwnode-gpiod-get-index
>
> I hope that it would be possible to pull in this immutable branch and
> not wait until after 5.5 merge window, or, alternatively, merge through
> Linus Walleij's tree.

Any chance this could be merged, please?

>
> Thanks!
>
> drivers/gpu/drm/bridge/ti-tfp410.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
> index aa3198dc9903..6f6d6d1e60ae 100644
> --- a/drivers/gpu/drm/bridge/ti-tfp410.c
> +++ b/drivers/gpu/drm/bridge/ti-tfp410.c
> @@ -285,8 +285,8 @@ static int tfp410_get_connector_properties(struct tfp410 *dvi)
> else
> dvi->connector_type = DRM_MODE_CONNECTOR_DVID;
>
> - dvi->hpd = fwnode_get_named_gpiod(&connector_node->fwnode,
> - "hpd-gpios", 0, GPIOD_IN, "hpd");
> + dvi->hpd = fwnode_gpiod_get_index(&connector_node->fwnode,
> + "hpd", 0, GPIOD_IN, "hpd");
> if (IS_ERR(dvi->hpd)) {
> ret = PTR_ERR(dvi->hpd);
> dvi->hpd = NULL;
> --
> 2.23.0.700.g56cf767bdb-goog
>
>
> --
> Dmitry

--
Dmitry

2019-11-05 15:31:36

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH] drm/bridge: ti-tfp410: switch to using fwnode_gpiod_get_index()

On Tue, Nov 5, 2019 at 1:40 AM Dmitry Torokhov
<[email protected]> wrote:
> On Mon, Oct 14, 2019 at 11:43:20AM -0700, Dmitry Torokhov wrote:

> > Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use
> > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), but
> > works with arbitrary firmware node.
> >
> > Reviewed-by: Laurent Pinchart <[email protected]>
> > Signed-off-by: Dmitry Torokhov <[email protected]>
> > ---
> >
> > Andrzej, Neil,
> >
> > This depends on the new code that can be bound in
> > ib-fwnode-gpiod-get-index immutable branch of Linus' Walleij tree:
> >
> > git pull git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git ib-fwnode-gpiod-get-index
> >
> > I hope that it would be possible to pull in this immutable branch and
> > not wait until after 5.5 merge window, or, alternatively, merge through
> > Linus Walleij's tree.
>
> Any chance this could be merged, please?

I'm happy to merge it into the GPIO tree if some DRM maintainer can
provide an ACK.

Getting ACK from DRM people is problematic and a bit of friction in the
community, DVetter usually advice to seek mutual reviews etc, but IMO
it would be better if some people felt more compelled to review stuff
eventually. (And that has the problem that it doesn't scale.)

Yours,
Linus Walleij

2019-11-05 15:43:36

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH] drm/bridge: ti-tfp410: switch to using fwnode_gpiod_get_index()

On Tue, Nov 5, 2019 at 4:29 PM Linus Walleij <[email protected]> wrote:
>
> On Tue, Nov 5, 2019 at 1:40 AM Dmitry Torokhov
> <[email protected]> wrote:
> > On Mon, Oct 14, 2019 at 11:43:20AM -0700, Dmitry Torokhov wrote:
>
> > > Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use
> > > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), but
> > > works with arbitrary firmware node.
> > >
> > > Reviewed-by: Laurent Pinchart <[email protected]>
> > > Signed-off-by: Dmitry Torokhov <[email protected]>
> > > ---
> > >
> > > Andrzej, Neil,
> > >
> > > This depends on the new code that can be bound in
> > > ib-fwnode-gpiod-get-index immutable branch of Linus' Walleij tree:
> > >
> > > git pull git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git ib-fwnode-gpiod-get-index
> > >
> > > I hope that it would be possible to pull in this immutable branch and
> > > not wait until after 5.5 merge window, or, alternatively, merge through
> > > Linus Walleij's tree.
> >
> > Any chance this could be merged, please?
>
> I'm happy to merge it into the GPIO tree if some DRM maintainer can
> provide an ACK.

Ack.

> Getting ACK from DRM people is problematic and a bit of friction in the
> community, DVetter usually advice to seek mutual reviews etc, but IMO
> it would be better if some people felt more compelled to review stuff
> eventually. (And that has the problem that it doesn't scale.)

This has a review already plus if you merge your implied review.
That's more than good enough imo, so not seeing the issue here?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

2019-11-05 19:54:47

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH] drm/bridge: ti-tfp410: switch to using fwnode_gpiod_get_index()

Hi Daniel,

On Tue, Nov 05, 2019 at 04:41:41PM +0100, Daniel Vetter wrote:
> On Tue, Nov 5, 2019 at 4:29 PM Linus Walleij wrote:
> > On Tue, Nov 5, 2019 at 1:40 AM Dmitry Torokhov wrote:
> > > On Mon, Oct 14, 2019 at 11:43:20AM -0700, Dmitry Torokhov wrote:
> >
> > > > Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use
> > > > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), but
> > > > works with arbitrary firmware node.
> > > >
> > > > Reviewed-by: Laurent Pinchart <[email protected]>
> > > > Signed-off-by: Dmitry Torokhov <[email protected]>
> > > > ---
> > > >
> > > > Andrzej, Neil,
> > > >
> > > > This depends on the new code that can be bound in
> > > > ib-fwnode-gpiod-get-index immutable branch of Linus' Walleij tree:
> > > >
> > > > git pull git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git ib-fwnode-gpiod-get-index
> > > >
> > > > I hope that it would be possible to pull in this immutable branch and
> > > > not wait until after 5.5 merge window, or, alternatively, merge through
> > > > Linus Walleij's tree.
> > >
> > > Any chance this could be merged, please?
> >
> > I'm happy to merge it into the GPIO tree if some DRM maintainer can
> > provide an ACK.
>
> Ack.
>
> > Getting ACK from DRM people is problematic and a bit of friction in the
> > community, DVetter usually advice to seek mutual reviews etc, but IMO
> > it would be better if some people felt more compelled to review stuff
> > eventually. (And that has the problem that it doesn't scale.)
>
> This has a review already plus if you merge your implied review.
> That's more than good enough imo, so not seeing the issue here?

Isn't the issue that the patch should have been picked by someone for
drm-misc ?

--
Regards,

Laurent Pinchart

2019-11-06 09:54:03

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH] drm/bridge: ti-tfp410: switch to using fwnode_gpiod_get_index()

On Tue, Nov 05, 2019 at 09:53:23PM +0200, Laurent Pinchart wrote:
> Hi Daniel,
>
> On Tue, Nov 05, 2019 at 04:41:41PM +0100, Daniel Vetter wrote:
> > On Tue, Nov 5, 2019 at 4:29 PM Linus Walleij wrote:
> > > On Tue, Nov 5, 2019 at 1:40 AM Dmitry Torokhov wrote:
> > > > On Mon, Oct 14, 2019 at 11:43:20AM -0700, Dmitry Torokhov wrote:
> > >
> > > > > Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use
> > > > > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), but
> > > > > works with arbitrary firmware node.
> > > > >
> > > > > Reviewed-by: Laurent Pinchart <[email protected]>
> > > > > Signed-off-by: Dmitry Torokhov <[email protected]>
> > > > > ---
> > > > >
> > > > > Andrzej, Neil,
> > > > >
> > > > > This depends on the new code that can be bound in
> > > > > ib-fwnode-gpiod-get-index immutable branch of Linus' Walleij tree:
> > > > >
> > > > > git pull git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git ib-fwnode-gpiod-get-index
> > > > >
> > > > > I hope that it would be possible to pull in this immutable branch and
> > > > > not wait until after 5.5 merge window, or, alternatively, merge through
> > > > > Linus Walleij's tree.
> > > >
> > > > Any chance this could be merged, please?
> > >
> > > I'm happy to merge it into the GPIO tree if some DRM maintainer can
> > > provide an ACK.
> >
> > Ack.
> >
> > > Getting ACK from DRM people is problematic and a bit of friction in the
> > > community, DVetter usually advice to seek mutual reviews etc, but IMO
> > > it would be better if some people felt more compelled to review stuff
> > > eventually. (And that has the problem that it doesn't scale.)
> >
> > This has a review already plus if you merge your implied review.
> > That's more than good enough imo, so not seeing the issue here?
>
> Isn't the issue that the patch should have been picked by someone for
> drm-misc ?

It requires prep work that isn't in drm-misc I thought? Anyway, Linus has
commit rights to drm-misc, could also push it there.

Plus you have, except you don't want it.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2019-11-13 15:55:39

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH] drm/bridge: ti-tfp410: switch to using fwnode_gpiod_get_index()

On Mon, Oct 14, 2019 at 8:43 PM Dmitry Torokhov
<[email protected]> wrote:

> Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use
> the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), but
> works with arbitrary firmware node.
>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Signed-off-by: Dmitry Torokhov <[email protected]>

I applied this with some ACKs to the GPIO devel branch for v5.5.

Yours,
Linus Walleij

2019-11-13 15:57:43

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH] drm/bridge: ti-tfp410: switch to using fwnode_gpiod_get_index()

On Tue, Nov 5, 2019 at 4:41 PM Daniel Vetter <[email protected]> wrote:
> On Tue, Nov 5, 2019 at 4:29 PM Linus Walleij <[email protected]> wrote:
> > On Tue, Nov 5, 2019 at 1:40 AM Dmitry Torokhov
> > <[email protected]> wrote:

> > I'm happy to merge it into the GPIO tree if some DRM maintainer can
> > provide an ACK.
>
> Ack.

Thanks!

> > Getting ACK from DRM people is problematic and a bit of friction in the
> > community, DVetter usually advice to seek mutual reviews etc, but IMO
> > it would be better if some people felt more compelled to review stuff
> > eventually. (And that has the problem that it doesn't scale.)
>
> This has a review already plus if you merge your implied review.

Yeah I missed Laurent's review tag. I needed some kund of consent
to take it into the GPIO tree I suppose.

> That's more than good enough imo, so not seeing the issue here?

No issue.

What freaked me out was the option of having to pull in an
immutable branch from my GPIO tree into drm-misc. That would
have been scary. Keeping it all in my tree works fine.

Yours,
Linus Walleij

2019-11-13 17:30:13

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH] drm/bridge: ti-tfp410: switch to using fwnode_gpiod_get_index()

On Wed, Nov 13, 2019 at 02:52:23PM +0100, Linus Walleij wrote:
> On Tue, Nov 5, 2019 at 4:41 PM Daniel Vetter <[email protected]> wrote:
> > On Tue, Nov 5, 2019 at 4:29 PM Linus Walleij <[email protected]> wrote:
> > > On Tue, Nov 5, 2019 at 1:40 AM Dmitry Torokhov
> > > <[email protected]> wrote:
>
> > > I'm happy to merge it into the GPIO tree if some DRM maintainer can
> > > provide an ACK.
> >
> > Ack.
>
> Thanks!
>
> > > Getting ACK from DRM people is problematic and a bit of friction in the
> > > community, DVetter usually advice to seek mutual reviews etc, but IMO
> > > it would be better if some people felt more compelled to review stuff
> > > eventually. (And that has the problem that it doesn't scale.)
> >
> > This has a review already plus if you merge your implied review.
>
> Yeah I missed Laurent's review tag. I needed some kund of consent
> to take it into the GPIO tree I suppose.
>
> > That's more than good enough imo, so not seeing the issue here?
>
> No issue.
>
> What freaked me out was the option of having to pull in an
> immutable branch from my GPIO tree into drm-misc. That would
> have been scary. Keeping it all in my tree works fine.

For next time around, just ping one of the drm-misc (or drm maintainers)
for an ack to merge it through a different tree. Or ask them what they
want to do. committer model isn't 100% free-wheeling, for cross-tree stuff
you still have maintainers who're supposed to do their jobs :-)

But by default everyone will assume you'll just commit, so you need to
poke them explicitly (like you've done here).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch