diff mbox

media: arch: sh: migor: Fix TW9910 PDN gpio

Message ID 1527671604-18768-1-git-send-email-jacopo+renesas@jmondi.org (mailing list archive)
State New, archived
Headers show

Commit Message

Jacopo Mondi May 30, 2018, 9:13 a.m. UTC
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(-)

--
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Geert Uytterhoeven May 30, 2018, 9:30 a.m. UTC | #1
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
Sergei Shtylyov May 30, 2018, 11:08 a.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laurent Pinchart May 30, 2018, 11:52 a.m. UTC | #3
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 ?
Jacopo Mondi May 30, 2018, 12:23 p.m. UTC | #4
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
>
>
>
Laurent Pinchart May 30, 2018, 12:38 p.m. UTC | #5
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 ? :-)
Jacopo Mondi May 30, 2018, 12:51 p.m. UTC | #6
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
>
>
>
diff mbox

Patch

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),
 	},
 };