[PATCH] media: arch: sh: migor: Fix TW9910 PDN gpio

[PATCH] media: arch: sh: migor: Fix TW9910 PDN gpio

[Jacopo Mondi]

The TW9910 PDN gpio (power down) is listed as active high in the chip
manual. It turns out it is actually active low as when set to physical
level 0 it actually turns the video decoder power off.

Without this patch applied:
tw9910 0-0045: Product ID error 1f:2

With this patch applied:
tw9910 0-0045: tw9910 Product ID b:0

Fixes: commit "186c446f4b840bd77b79d3dc951ca436cb8abe79"

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

---
Hi,
   sending to both media and sh lists, as all previous CEU-related patches
went through Hans' tree, even the board specific parts.

Thanks
   j
---
 arch/sh/boards/mach-migor/setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c
index 271dfc2..3d7d004 100644
--- a/arch/sh/boards/mach-migor/setup.c
+++ b/arch/sh/boards/mach-migor/setup.c
@@ -359,7 +359,7 @@ static struct gpiod_lookup_table ov7725_gpios = {
 static struct gpiod_lookup_table tw9910_gpios = {
 	.dev_id		= "0-0045",
 	.table		= {
-		GPIO_LOOKUP("sh7722_pfc", GPIO_PTT2, "pdn", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("sh7722_pfc", GPIO_PTT2, "pdn", GPIO_ACTIVE_LOW),
 		GPIO_LOOKUP("sh7722_pfc", GPIO_PTT3, "rstb", GPIO_ACTIVE_LOW),
 	},
 };
--
2.7.4

[PATCH] media: arch: sh: migor: Fix TW9910 PDN gpio

[Jacopo Mondi]

The TW9910 PDN gpio (power down) is listed as active high in the chip
manual. It turns out it is actually active low as when set to physical
level 0 it actually turns the video decoder power off.

Without this patch applied:
tw9910 0-0045: Product ID error 1f:2

With this patch applied:
tw9910 0-0045: tw9910 Product ID b:0

Fixes: commit "186c446f4b840bd77b79d3dc951ca436cb8abe79"

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

---
Hi,
   sending to both media and sh lists, as all previous CEU-related patches
went through Hans' tree, even the board specific parts.

Thanks
   j
---
 arch/sh/boards/mach-migor/setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c
index 271dfc2..3d7d004 100644
--- a/arch/sh/boards/mach-migor/setup.c
+++ b/arch/sh/boards/mach-migor/setup.c
@@ -359,7 +359,7 @@ static struct gpiod_lookup_table ov7725_gpios = {
 static struct gpiod_lookup_table tw9910_gpios = {
 	.dev_id		= "0-0045",
 	.table		= {
-		GPIO_LOOKUP("sh7722_pfc", GPIO_PTT2, "pdn", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("sh7722_pfc", GPIO_PTT2, "pdn", GPIO_ACTIVE_LOW),
 		GPIO_LOOKUP("sh7722_pfc", GPIO_PTT3, "rstb", GPIO_ACTIVE_LOW),
 	},
 };
--
2.7.4

Re: [PATCH] media: arch: sh: migor: Fix TW9910 PDN gpio

[Geert Uytterhoeven]

Hi Jacopo,

On Wed, May 30, 2018 at 11:13 AM, Jacopo Mondi
<jacopo+renesas@jmondi.org> wrote:
> The TW9910 PDN gpio (power down) is listed as active high in the chip
> manual. It turns out it is actually active low as when set to physical
> level 0 it actually turns the video decoder power off.

So the picture "Typical TW9910 External Circuitry" in the datasheet, which
ties PDN to GND permanently, is wrong?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] media: arch: sh: migor: Fix TW9910 PDN gpio

[Geert Uytterhoeven]

Hi Jacopo,

On Wed, May 30, 2018 at 11:13 AM, Jacopo Mondi
<jacopo+renesas@jmondi.org> wrote:
> The TW9910 PDN gpio (power down) is listed as active high in the chip
> manual. It turns out it is actually active low as when set to physical
> level 0 it actually turns the video decoder power off.

So the picture "Typical TW9910 External Circuitry" in the datasheet, which
ties PDN to GND permanently, is wrong?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] media: arch: sh: migor: Fix TW9910 PDN gpio

[Laurent Pinchart]

Hi Geert,

On Wednesday, 30 May 2018 12:30:49 EEST Geert Uytterhoeven wrote:
> Hi Jacopo,
> 
> On Wed, May 30, 2018 at 11:13 AM, Jacopo Mondi wrote:
> > The TW9910 PDN gpio (power down) is listed as active high in the chip
> > manual. It turns out it is actually active low as when set to physical
> > level 0 it actually turns the video decoder power off.
> 
> So the picture "Typical TW9910 External Circuitry" in the datasheet, which
> ties PDN to GND permanently, is wrong?

The SH PTT2 line is connected directory to the TW9910 PDN signal, without any 
inverter on the board. The PDN signal is clearly documented as active-high in 
the TW9910 datasheet. Something is thus weird.

Jacopo, is it possible to measure the PDN signal on the board as close as 
possible to the TW9910 to see if it works as expected ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: arch: sh: migor: Fix TW9910 PDN gpio

[Laurent Pinchart]

Hi Geert,

On Wednesday, 30 May 2018 12:30:49 EEST Geert Uytterhoeven wrote:
> Hi Jacopo,
> 
> On Wed, May 30, 2018 at 11:13 AM, Jacopo Mondi wrote:
> > The TW9910 PDN gpio (power down) is listed as active high in the chip
> > manual. It turns out it is actually active low as when set to physical
> > level 0 it actually turns the video decoder power off.
> 
> So the picture "Typical TW9910 External Circuitry" in the datasheet, which
> ties PDN to GND permanently, is wrong?

The SH PTT2 line is connected directory to the TW9910 PDN signal, without any 
inverter on the board. The PDN signal is clearly documented as active-high in 
the TW9910 datasheet. Something is thus weird.

Jacopo, is it possible to measure the PDN signal on the board as close as 
possible to the TW9910 to see if it works as expected ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: arch: sh: migor: Fix TW9910 PDN gpio

[jacopo mondi]

[-- Attachment #1: Type: text/plain, Size: 1695 bytes --]

Hi Laurent, Geert,

On Wed, May 30, 2018 at 02:52:31PM +0300, Laurent Pinchart wrote:
> Hi Geert,
>
> On Wednesday, 30 May 2018 12:30:49 EEST Geert Uytterhoeven wrote:
> > Hi Jacopo,
> >
> > On Wed, May 30, 2018 at 11:13 AM, Jacopo Mondi wrote:
> > > The TW9910 PDN gpio (power down) is listed as active high in the chip
> > > manual. It turns out it is actually active low as when set to physical
> > > level 0 it actually turns the video decoder power off.
> >
> > So the picture "Typical TW9910 External Circuitry" in the datasheet, which
> > ties PDN to GND permanently, is wrong?

Also the definition of PDN pin in TW9910 manual, as reported by Laurent made me
think the pin had to stay in logical state 1 to have the chip powered
down. That's why my initial 'ACTIVE_HIGH' flag. The chip was not
recognized, but I thought it was a local problem of the Migo-R board I
was using.

Then one day I tried inverting the pin active state just to be sure,
and it started being fully operational :/

>
> The SH PTT2 line is connected directory to the TW9910 PDN signal, without any
> inverter on the board. The PDN signal is clearly documented as active-high in
> the TW9910 datasheet. Something is thus weird.

I suspect the 'active high' definition in datasheet is different from
our understanding. Their 'active' means the chip is operational, which
is not what one would expect from a powerdown pin.

>
> Jacopo, is it possible to measure the PDN signal on the board as close as
> possible to the TW9910 to see if it works as expected ?

Not for me. The board is in Japan and my multimeter doesn't have cables
that long, unfortunately.

Thanks
   j

>
> --
> Regards,
>
> Laurent Pinchart
>
>
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] media: arch: sh: migor: Fix TW9910 PDN gpio

[jacopo mondi]

[-- Attachment #1: Type: text/plain, Size: 1695 bytes --]

Hi Laurent, Geert,

On Wed, May 30, 2018 at 02:52:31PM +0300, Laurent Pinchart wrote:
> Hi Geert,
>
> On Wednesday, 30 May 2018 12:30:49 EEST Geert Uytterhoeven wrote:
> > Hi Jacopo,
> >
> > On Wed, May 30, 2018 at 11:13 AM, Jacopo Mondi wrote:
> > > The TW9910 PDN gpio (power down) is listed as active high in the chip
> > > manual. It turns out it is actually active low as when set to physical
> > > level 0 it actually turns the video decoder power off.
> >
> > So the picture "Typical TW9910 External Circuitry" in the datasheet, which
> > ties PDN to GND permanently, is wrong?

Also the definition of PDN pin in TW9910 manual, as reported by Laurent made me
think the pin had to stay in logical state 1 to have the chip powered
down. That's why my initial 'ACTIVE_HIGH' flag. The chip was not
recognized, but I thought it was a local problem of the Migo-R board I
was using.

Then one day I tried inverting the pin active state just to be sure,
and it started being fully operational :/

>
> The SH PTT2 line is connected directory to the TW9910 PDN signal, without any
> inverter on the board. The PDN signal is clearly documented as active-high in
> the TW9910 datasheet. Something is thus weird.

I suspect the 'active high' definition in datasheet is different from
our understanding. Their 'active' means the chip is operational, which
is not what one would expect from a powerdown pin.

>
> Jacopo, is it possible to measure the PDN signal on the board as close as
> possible to the TW9910 to see if it works as expected ?

Not for me. The board is in Japan and my multimeter doesn't have cables
that long, unfortunately.

Thanks
   j

>
> --
> Regards,
>
> Laurent Pinchart
>
>
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] media: arch: sh: migor: Fix TW9910 PDN gpio

[Laurent Pinchart]

Hi Jacopo,

On Wednesday, 30 May 2018 15:23:43 EEST jacopo mondi wrote:
> On Wed, May 30, 2018 at 02:52:31PM +0300, Laurent Pinchart wrote:
> > On Wednesday, 30 May 2018 12:30:49 EEST Geert Uytterhoeven wrote:
> >> On Wed, May 30, 2018 at 11:13 AM, Jacopo Mondi wrote:
> >>> The TW9910 PDN gpio (power down) is listed as active high in the chip
> >>> manual. It turns out it is actually active low as when set to physical
> >>> level 0 it actually turns the video decoder power off.
> >> 
> >> So the picture "Typical TW9910 External Circuitry" in the datasheet,
> >> which ties PDN to GND permanently, is wrong?
> 
> Also the definition of PDN pin in TW9910 manual, as reported by Laurent made
> me think the pin had to stay in logical state 1 to have the chip powered
> down. That's why my initial 'ACTIVE_HIGH' flag. The chip was not
> recognized, but I thought it was a local problem of the Migo-R board I
> was using.
> 
> Then one day I tried inverting the pin active state just to be sure,
> and it started being fully operational :/
> 
> > The SH PTT2 line is connected directory to the TW9910 PDN signal, without
> > any inverter on the board. The PDN signal is clearly documented as
> > active-high in the TW9910 datasheet. Something is thus weird.
> 
> I suspect the 'active high' definition in datasheet is different from
> our understanding. Their 'active' means the chip is operational, which
> is not what one would expect from a powerdown pin.
> 
> > Jacopo, is it possible to measure the PDN signal on the board as close as
> > possible to the TW9910 to see if it works as expected ?
> 
> Not for me. The board is in Japan and my multimeter doesn't have cables
> that long, unfortunately.

How about trying that during your next trip to Japan ? :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: arch: sh: migor: Fix TW9910 PDN gpio

[Laurent Pinchart]

Hi Jacopo,

On Wednesday, 30 May 2018 15:23:43 EEST jacopo mondi wrote:
> On Wed, May 30, 2018 at 02:52:31PM +0300, Laurent Pinchart wrote:
> > On Wednesday, 30 May 2018 12:30:49 EEST Geert Uytterhoeven wrote:
> >> On Wed, May 30, 2018 at 11:13 AM, Jacopo Mondi wrote:
> >>> The TW9910 PDN gpio (power down) is listed as active high in the chip
> >>> manual. It turns out it is actually active low as when set to physical
> >>> level 0 it actually turns the video decoder power off.
> >> 
> >> So the picture "Typical TW9910 External Circuitry" in the datasheet,
> >> which ties PDN to GND permanently, is wrong?
> 
> Also the definition of PDN pin in TW9910 manual, as reported by Laurent made
> me think the pin had to stay in logical state 1 to have the chip powered
> down. That's why my initial 'ACTIVE_HIGH' flag. The chip was not
> recognized, but I thought it was a local problem of the Migo-R board I
> was using.
> 
> Then one day I tried inverting the pin active state just to be sure,
> and it started being fully operational :/
> 
> > The SH PTT2 line is connected directory to the TW9910 PDN signal, without
> > any inverter on the board. The PDN signal is clearly documented as
> > active-high in the TW9910 datasheet. Something is thus weird.
> 
> I suspect the 'active high' definition in datasheet is different from
> our understanding. Their 'active' means the chip is operational, which
> is not what one would expect from a powerdown pin.
> 
> > Jacopo, is it possible to measure the PDN signal on the board as close as
> > possible to the TW9910 to see if it works as expected ?
> 
> Not for me. The board is in Japan and my multimeter doesn't have cables
> that long, unfortunately.

How about trying that during your next trip to Japan ? :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: arch: sh: migor: Fix TW9910 PDN gpio

[jacopo mondi]

[-- Attachment #1: Type: text/plain, Size: 2014 bytes --]

Hi Laurent,

On Wed, May 30, 2018 at 03:38:01PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Wednesday, 30 May 2018 15:23:43 EEST jacopo mondi wrote:
> > On Wed, May 30, 2018 at 02:52:31PM +0300, Laurent Pinchart wrote:
> > > On Wednesday, 30 May 2018 12:30:49 EEST Geert Uytterhoeven wrote:
> > >> On Wed, May 30, 2018 at 11:13 AM, Jacopo Mondi wrote:
> > >>> The TW9910 PDN gpio (power down) is listed as active high in the chip
> > >>> manual. It turns out it is actually active low as when set to physical
> > >>> level 0 it actually turns the video decoder power off.
> > >>
> > >> So the picture "Typical TW9910 External Circuitry" in the datasheet,
> > >> which ties PDN to GND permanently, is wrong?
> >
> > Also the definition of PDN pin in TW9910 manual, as reported by Laurent made
> > me think the pin had to stay in logical state 1 to have the chip powered
> > down. That's why my initial 'ACTIVE_HIGH' flag. The chip was not
> > recognized, but I thought it was a local problem of the Migo-R board I
> > was using.
> >
> > Then one day I tried inverting the pin active state just to be sure,
> > and it started being fully operational :/
> >
> > > The SH PTT2 line is connected directory to the TW9910 PDN signal, without
> > > any inverter on the board. The PDN signal is clearly documented as
> > > active-high in the TW9910 datasheet. Something is thus weird.
> >
> > I suspect the 'active high' definition in datasheet is different from
> > our understanding. Their 'active' means the chip is operational, which
> > is not what one would expect from a powerdown pin.
> >
> > > Jacopo, is it possible to measure the PDN signal on the board as close as
> > > possible to the TW9910 to see if it works as expected ?
> >
> > Not for me. The board is in Japan and my multimeter doesn't have cables
> > that long, unfortunately.
>
> How about trying that during your next trip to Japan ? :-)

Ok, I'll stop looking for cables 10.000km long on amazon.
>
> --
> Regards,
>
> Laurent Pinchart
>
>
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] media: arch: sh: migor: Fix TW9910 PDN gpio

[jacopo mondi]

[-- Attachment #1: Type: text/plain, Size: 2014 bytes --]

Hi Laurent,

On Wed, May 30, 2018 at 03:38:01PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Wednesday, 30 May 2018 15:23:43 EEST jacopo mondi wrote:
> > On Wed, May 30, 2018 at 02:52:31PM +0300, Laurent Pinchart wrote:
> > > On Wednesday, 30 May 2018 12:30:49 EEST Geert Uytterhoeven wrote:
> > >> On Wed, May 30, 2018 at 11:13 AM, Jacopo Mondi wrote:
> > >>> The TW9910 PDN gpio (power down) is listed as active high in the chip
> > >>> manual. It turns out it is actually active low as when set to physical
> > >>> level 0 it actually turns the video decoder power off.
> > >>
> > >> So the picture "Typical TW9910 External Circuitry" in the datasheet,
> > >> which ties PDN to GND permanently, is wrong?
> >
> > Also the definition of PDN pin in TW9910 manual, as reported by Laurent made
> > me think the pin had to stay in logical state 1 to have the chip powered
> > down. That's why my initial 'ACTIVE_HIGH' flag. The chip was not
> > recognized, but I thought it was a local problem of the Migo-R board I
> > was using.
> >
> > Then one day I tried inverting the pin active state just to be sure,
> > and it started being fully operational :/
> >
> > > The SH PTT2 line is connected directory to the TW9910 PDN signal, without
> > > any inverter on the board. The PDN signal is clearly documented as
> > > active-high in the TW9910 datasheet. Something is thus weird.
> >
> > I suspect the 'active high' definition in datasheet is different from
> > our understanding. Their 'active' means the chip is operational, which
> > is not what one would expect from a powerdown pin.
> >
> > > Jacopo, is it possible to measure the PDN signal on the board as close as
> > > possible to the TW9910 to see if it works as expected ?
> >
> > Not for me. The board is in Japan and my multimeter doesn't have cables
> > that long, unfortunately.
>
> How about trying that during your next trip to Japan ? :-)

Ok, I'll stop looking for cables 10.000km long on amazon.
>
> --
> Regards,
>
> Laurent Pinchart
>
>
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] media: arch: sh: migor: Fix TW9910 PDN gpio

[Sergei Shtylyov]

Hello!

On 05/30/2018 12:13 PM, Jacopo Mondi wrote:

> The TW9910 PDN gpio (power down) is listed as active high in the chip

   GPIO.

> manual. It turns out it is actually active low as when set to physical
> level 0 it actually turns the video decoder power off.
> 
> Without this patch applied:
> tw9910 0-0045: Product ID error 1f:2
> 
> With this patch applied:
> tw9910 0-0045: tw9910 Product ID b:0
> 
> Fixes: commit "186c446f4b840bd77b79d3dc951ca436cb8abe79"

   That's not how you specify the "Fixes:" tag, please see
Documentation/process/submitting-patches.rst.

> 

   There shouldn't be an emoty line here.

> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
[...]

MBR, Sergei

Re: [PATCH] media: arch: sh: migor: Fix TW9910 PDN gpio

[Sergei Shtylyov]

Hello!

On 05/30/2018 12:13 PM, Jacopo Mondi wrote:

> The TW9910 PDN gpio (power down) is listed as active high in the chip

   GPIO.

> manual. It turns out it is actually active low as when set to physical
> level 0 it actually turns the video decoder power off.
> 
> Without this patch applied:
> tw9910 0-0045: Product ID error 1f:2
> 
> With this patch applied:
> tw9910 0-0045: tw9910 Product ID b:0
> 
> Fixes: commit "186c446f4b840bd77b79d3dc951ca436cb8abe79"

   That's not how you specify the "Fixes:" tag, please see
Documentation/process/submitting-patches.rst.

> 

   There shouldn't be an emoty line here.

> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
[...]

MBR, Sergei