| Message ID | 1487771027-7763-4-git-send-email-bgolaszewski@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Feb 22, 2017 at 02:43:47PM +0100, Bartosz Golaszewski wrote: > Add an optional property - enable-gpios - which can be used to specify > the GPIOs that must be requested to select the vpif functionality. > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > --- > Documentation/devicetree/bindings/media/ti,da850-vpif.txt | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt > index df7182a..23c5405 100644 > --- a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt > +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt > @@ -13,6 +13,9 @@ Required properties: > - reg: physical base address and length of the registers set for the device; > - interrupts: should contain IRQ line for the VPIF > > +Optional properties: > +- enable-gpios: phandle of the GPIOs used to select the vpif functionality What does this control exactly? The GPIOs belong in the node they are connected to and having GPIOs routed to this block seems strange. There are inputs or outputs? > + > Video Capture: > > VPIF has a 16-bit parallel bus input, supporting 2 8-bit channels or a > @@ -30,6 +33,10 @@ I2C-connected TVP5147 decoder: > reg = <0x217000 0x1000>; > interrupts = <92>; > > + enable-gpios = <&tca6416 7 GPIO_ACTIVE_HIGH > + &tca6416 6 GPIO_ACTIVE_HIGH > + &tca6416 5 GPIO_ACTIVE_LOW>; > + > port@0 { > vpif_input_ch0: endpoint@0 { > reg = <0>; > -- > 2.9.3 >
On Tuesday 28 February 2017 04:22 AM, Rob Herring wrote: > On Wed, Feb 22, 2017 at 02:43:47PM +0100, Bartosz Golaszewski wrote: >> Add an optional property - enable-gpios - which can be used to specify >> the GPIOs that must be requested to select the vpif functionality. >> >> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> >> --- >> Documentation/devicetree/bindings/media/ti,da850-vpif.txt | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt >> index df7182a..23c5405 100644 >> --- a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt >> +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt >> @@ -13,6 +13,9 @@ Required properties: >> - reg: physical base address and length of the registers set for the device; >> - interrupts: should contain IRQ line for the VPIF >> >> +Optional properties: >> +- enable-gpios: phandle of the GPIOs used to select the vpif functionality > > What does this control exactly? The GPIOs belong in the node they are > connected to and having GPIOs routed to this block seems strange. The DA850 EVM board implements on-board muxing which lets the video input (via VPIF) to be routed to a TVP5147 (video decoder) for composite input or to a camera header. There are other mux options which use the same VPIF SoC pins (RMII ethernet or character LCD). There is a three-to-eight demux on the board which drives enable signals to buffers letting you choose from these options. From your response, it looks like you want the enable-gpios property to be in the TVP5147 or camera node. That does make sense to me. > > There are inputs or outputs? These GPIOs are driving a 3-to-8 demux so they are all outputs. Thanks, Sekhar
2017-02-28 6:36 GMT+01:00 Sekhar Nori <nsekhar@ti.com>: > On Tuesday 28 February 2017 04:22 AM, Rob Herring wrote: >> On Wed, Feb 22, 2017 at 02:43:47PM +0100, Bartosz Golaszewski wrote: >>> Add an optional property - enable-gpios - which can be used to specify >>> the GPIOs that must be requested to select the vpif functionality. >>> >>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> >>> --- >>> Documentation/devicetree/bindings/media/ti,da850-vpif.txt | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt >>> index df7182a..23c5405 100644 >>> --- a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt >>> +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt >>> @@ -13,6 +13,9 @@ Required properties: >>> - reg: physical base address and length of the registers set for the device; >>> - interrupts: should contain IRQ line for the VPIF >>> >>> +Optional properties: >>> +- enable-gpios: phandle of the GPIOs used to select the vpif functionality >> >> What does this control exactly? The GPIOs belong in the node they are >> connected to and having GPIOs routed to this block seems strange. > > The DA850 EVM board implements on-board muxing which lets the video > input (via VPIF) to be routed to a TVP5147 (video decoder) for composite > input or to a camera header. There are other mux options which use the > same VPIF SoC pins (RMII ethernet or character LCD). > > There is a three-to-eight demux on the board which drives enable signals > to buffers letting you choose from these options. > > From your response, it looks like you want the enable-gpios property to > be in the TVP5147 or camera node. That does make sense to me. > Wouldn't that cause races again? The TVP5147 ADC is probed from within the vpif driver as a media subdev (after initialize_vpif(), v4l2_device_register(), irq requests etc.). Thanks, Bartosz
On Mon, Feb 27, 2017 at 11:36 PM, Sekhar Nori <nsekhar@ti.com> wrote: > On Tuesday 28 February 2017 04:22 AM, Rob Herring wrote: >> On Wed, Feb 22, 2017 at 02:43:47PM +0100, Bartosz Golaszewski wrote: >>> Add an optional property - enable-gpios - which can be used to specify >>> the GPIOs that must be requested to select the vpif functionality. >>> >>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> >>> --- >>> Documentation/devicetree/bindings/media/ti,da850-vpif.txt | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt >>> index df7182a..23c5405 100644 >>> --- a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt >>> +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt >>> @@ -13,6 +13,9 @@ Required properties: >>> - reg: physical base address and length of the registers set for the device; >>> - interrupts: should contain IRQ line for the VPIF >>> >>> +Optional properties: >>> +- enable-gpios: phandle of the GPIOs used to select the vpif functionality >> >> What does this control exactly? The GPIOs belong in the node they are >> connected to and having GPIOs routed to this block seems strange. > > The DA850 EVM board implements on-board muxing which lets the video > input (via VPIF) to be routed to a TVP5147 (video decoder) for composite > input or to a camera header. There are other mux options which use the > same VPIF SoC pins (RMII ethernet or character LCD). > > There is a three-to-eight demux on the board which drives enable signals > to buffers letting you choose from these options. > > From your response, it looks like you want the enable-gpios property to > be in the TVP5147 or camera node. That does make sense to me. Well, seems like they should be part of a mux node. I'd suggest you look at the video-multiplexer binding under review[1]. Though for other non video functions, that would be some sort of board level pin mux control. There's a new mux binding too, maybe that would work. In any case, it shouldn't be the VPIF driver controlling the GPIOs directly. Rob [1] https://lkml.org/lkml/2017/1/6/1056
On Tuesday 28 February 2017 07:56 PM, Rob Herring wrote: > On Mon, Feb 27, 2017 at 11:36 PM, Sekhar Nori <nsekhar@ti.com> wrote: >> On Tuesday 28 February 2017 04:22 AM, Rob Herring wrote: >>> On Wed, Feb 22, 2017 at 02:43:47PM +0100, Bartosz Golaszewski wrote: >>>> Add an optional property - enable-gpios - which can be used to specify >>>> the GPIOs that must be requested to select the vpif functionality. >>>> >>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> >>>> --- >>>> Documentation/devicetree/bindings/media/ti,da850-vpif.txt | 7 +++++++ >>>> 1 file changed, 7 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt >>>> index df7182a..23c5405 100644 >>>> --- a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt >>>> +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt >>>> @@ -13,6 +13,9 @@ Required properties: >>>> - reg: physical base address and length of the registers set for the device; >>>> - interrupts: should contain IRQ line for the VPIF >>>> >>>> +Optional properties: >>>> +- enable-gpios: phandle of the GPIOs used to select the vpif functionality >>> >>> What does this control exactly? The GPIOs belong in the node they are >>> connected to and having GPIOs routed to this block seems strange. >> >> The DA850 EVM board implements on-board muxing which lets the video >> input (via VPIF) to be routed to a TVP5147 (video decoder) for composite >> input or to a camera header. There are other mux options which use the >> same VPIF SoC pins (RMII ethernet or character LCD). >> >> There is a three-to-eight demux on the board which drives enable signals >> to buffers letting you choose from these options. >> >> From your response, it looks like you want the enable-gpios property to >> be in the TVP5147 or camera node. That does make sense to me. > > Well, seems like they should be part of a mux node. I'd suggest you > look at the video-multiplexer binding under review[1]. > > Though for other non video functions, that would be some sort of board > level pin mux control. There's a new mux binding too, maybe that would > work. In any case, it shouldn't be the VPIF driver controlling the > GPIOs directly. Yeah, since there are non-video functions involved, we need a more generic mux binding. I see the work Peter Rosin is doing[1]. Its not in linux-next yet, but seems quite near merging. [1] https://lkml.org/lkml/2017/2/8/394 Thanks, Sekhar
On Tuesday 28 February 2017 04:30 PM, Bartosz Golaszewski wrote: > 2017-02-28 6:36 GMT+01:00 Sekhar Nori <nsekhar@ti.com>: >> On Tuesday 28 February 2017 04:22 AM, Rob Herring wrote: >>> On Wed, Feb 22, 2017 at 02:43:47PM +0100, Bartosz Golaszewski wrote: >>>> Add an optional property - enable-gpios - which can be used to specify >>>> the GPIOs that must be requested to select the vpif functionality. >>>> >>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> >>>> --- >>>> Documentation/devicetree/bindings/media/ti,da850-vpif.txt | 7 +++++++ >>>> 1 file changed, 7 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt >>>> index df7182a..23c5405 100644 >>>> --- a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt >>>> +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt >>>> @@ -13,6 +13,9 @@ Required properties: >>>> - reg: physical base address and length of the registers set for the device; >>>> - interrupts: should contain IRQ line for the VPIF >>>> >>>> +Optional properties: >>>> +- enable-gpios: phandle of the GPIOs used to select the vpif functionality >>> >>> What does this control exactly? The GPIOs belong in the node they are >>> connected to and having GPIOs routed to this block seems strange. >> >> The DA850 EVM board implements on-board muxing which lets the video >> input (via VPIF) to be routed to a TVP5147 (video decoder) for composite >> input or to a camera header. There are other mux options which use the >> same VPIF SoC pins (RMII ethernet or character LCD). >> >> There is a three-to-eight demux on the board which drives enable signals >> to buffers letting you choose from these options. >> >> From your response, it looks like you want the enable-gpios property to >> be in the TVP5147 or camera node. That does make sense to me. >> > > Wouldn't that cause races again? The TVP5147 ADC is probed from within > the vpif driver as a media subdev (after initialize_vpif(), > v4l2_device_register(), irq requests etc.). When TVP5147 probe fails with -EPROBE_DEFER, I assume the error can propagate back to vpif_probe() which would then de-register the V4L2 device as part of probe error unwinding. That should then allow the probe to be retried at a later time. Thanks, Sekhar > > Thanks, > Bartosz >
diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt index df7182a..23c5405 100644 --- a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt @@ -13,6 +13,9 @@ Required properties: - reg: physical base address and length of the registers set for the device; - interrupts: should contain IRQ line for the VPIF +Optional properties: +- enable-gpios: phandle of the GPIOs used to select the vpif functionality + Video Capture: VPIF has a 16-bit parallel bus input, supporting 2 8-bit channels or a @@ -30,6 +33,10 @@ I2C-connected TVP5147 decoder: reg = <0x217000 0x1000>; interrupts = <92>; + enable-gpios = <&tca6416 7 GPIO_ACTIVE_HIGH + &tca6416 6 GPIO_ACTIVE_HIGH + &tca6416 5 GPIO_ACTIVE_LOW>; + port@0 { vpif_input_ch0: endpoint@0 { reg = <0>;
Add an optional property - enable-gpios - which can be used to specify the GPIOs that must be requested to select the vpif functionality. Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> --- Documentation/devicetree/bindings/media/ti,da850-vpif.txt | 7 +++++++ 1 file changed, 7 insertions(+)