[PATCH] sh-pfc: r8a7779: fixup INDTx address

[PATCH] sh-pfc: r8a7779: fixup INDTx address

[Kuninori Morimoto]

INDTn address is 0xFFC4x000C. 0xFFC4x0008 is OUTDTn

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/pinctrl/sh-pfc/pfc-r8a7779.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7779.c b/drivers/pinctrl/sh-pfc/pfc-r8a7779.c
index 13feaa0..cb97fb6 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7779.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7779.c
@@ -2585,13 +2585,13 @@ static struct pinmux_cfg_reg pinmux_config_regs[] = {
 };
 
 static struct pinmux_data_reg pinmux_data_regs[] = {
-	{ PINMUX_DATA_REG("INDT0", 0xffc40008, 32) { GP_INDT(0) } },
-	{ PINMUX_DATA_REG("INDT1", 0xffc41008, 32) { GP_INDT(1) } },
-	{ PINMUX_DATA_REG("INDT2", 0xffc42008, 32) { GP_INDT(2) } },
-	{ PINMUX_DATA_REG("INDT3", 0xffc43008, 32) { GP_INDT(3) } },
-	{ PINMUX_DATA_REG("INDT4", 0xffc44008, 32) { GP_INDT(4) } },
-	{ PINMUX_DATA_REG("INDT5", 0xffc45008, 32) { GP_INDT(5) } },
-	{ PINMUX_DATA_REG("INDT6", 0xffc46008, 32) {
+	{ PINMUX_DATA_REG("INDT0", 0xffc4000c, 32) { GP_INDT(0) } },
+	{ PINMUX_DATA_REG("INDT1", 0xffc4100c, 32) { GP_INDT(1) } },
+	{ PINMUX_DATA_REG("INDT2", 0xffc4200c, 32) { GP_INDT(2) } },
+	{ PINMUX_DATA_REG("INDT3", 0xffc4300c, 32) { GP_INDT(3) } },
+	{ PINMUX_DATA_REG("INDT4", 0xffc4400c, 32) { GP_INDT(4) } },
+	{ PINMUX_DATA_REG("INDT5", 0xffc4500c, 32) { GP_INDT(5) } },
+	{ PINMUX_DATA_REG("INDT6", 0xffc4600c, 32) {
 		0, 0, 0, 0, 0, 0, 0, 0,	0, 0, 0, 0, 0, 0, 0, 0,
 		0, 0, 0, 0, 0, 0, 0, GP_6_8_DATA,
 		GP_6_7_DATA, GP_6_6_DATA, GP_6_5_DATA, GP_6_4_DATA,
-- 
1.7.9.5

Re: [PATCH] sh-pfc: r8a7779: fixup INDTx address

[Simon Horman]

[ Cc: Linus Walleij ]

Hi Linus, Hi Paul, Hi Laurent,

I'm a little unsure how everyone would like to handle
drivers/pinctrl/sh-pfc/ updates.

I'm happy to create a pfc branch and send pull requests to Linus.
Or for Paul to handle things. Or for Laurent to handle things.
Or for other suggestions.

To confuse things, I believe there are a batch of PFC changes that
will make sense to take through the arm-soc tree due to dependencies.


Morimoto-san,

could you comment on how critical this change is.
Does anything in 3.9-rc1 depend on it?

On Mon, Mar 04, 2013 at 12:45:39AM -0800, Kuninori Morimoto wrote:
> INDTn address is 0xFFC4x000C. 0xFFC4x0008 is OUTDTn
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  drivers/pinctrl/sh-pfc/pfc-r8a7779.c |   14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7779.c b/drivers/pinctrl/sh-pfc/pfc-r8a7779.c
> index 13feaa0..cb97fb6 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7779.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7779.c
> @@ -2585,13 +2585,13 @@ static struct pinmux_cfg_reg pinmux_config_regs[] = {
>  };
>  
>  static struct pinmux_data_reg pinmux_data_regs[] = {
> -	{ PINMUX_DATA_REG("INDT0", 0xffc40008, 32) { GP_INDT(0) } },
> -	{ PINMUX_DATA_REG("INDT1", 0xffc41008, 32) { GP_INDT(1) } },
> -	{ PINMUX_DATA_REG("INDT2", 0xffc42008, 32) { GP_INDT(2) } },
> -	{ PINMUX_DATA_REG("INDT3", 0xffc43008, 32) { GP_INDT(3) } },
> -	{ PINMUX_DATA_REG("INDT4", 0xffc44008, 32) { GP_INDT(4) } },
> -	{ PINMUX_DATA_REG("INDT5", 0xffc45008, 32) { GP_INDT(5) } },
> -	{ PINMUX_DATA_REG("INDT6", 0xffc46008, 32) {
> +	{ PINMUX_DATA_REG("INDT0", 0xffc4000c, 32) { GP_INDT(0) } },
> +	{ PINMUX_DATA_REG("INDT1", 0xffc4100c, 32) { GP_INDT(1) } },
> +	{ PINMUX_DATA_REG("INDT2", 0xffc4200c, 32) { GP_INDT(2) } },
> +	{ PINMUX_DATA_REG("INDT3", 0xffc4300c, 32) { GP_INDT(3) } },
> +	{ PINMUX_DATA_REG("INDT4", 0xffc4400c, 32) { GP_INDT(4) } },
> +	{ PINMUX_DATA_REG("INDT5", 0xffc4500c, 32) { GP_INDT(5) } },
> +	{ PINMUX_DATA_REG("INDT6", 0xffc4600c, 32) {
>  		0, 0, 0, 0, 0, 0, 0, 0,	0, 0, 0, 0, 0, 0, 0, 0,
>  		0, 0, 0, 0, 0, 0, 0, GP_6_8_DATA,
>  		GP_6_7_DATA, GP_6_6_DATA, GP_6_5_DATA, GP_6_4_DATA,
> -- 
> 1.7.9.5
> 
> --
> 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
> 

Re: [PATCH] sh-pfc: r8a7779: fixup INDTx address

[Paul Mundt]

On Tue, Mar 05, 2013 at 12:18:52PM +0900, Simon Horman wrote:
> [ Cc: Linus Walleij ]
> 
> Hi Linus, Hi Paul, Hi Laurent,
> 
> I'm a little unsure how everyone would like to handle
> drivers/pinctrl/sh-pfc/ updates.
> 
> I'm happy to create a pfc branch and send pull requests to Linus.
> Or for Paul to handle things. Or for Laurent to handle things.
> Or for other suggestions.
> 
> To confuse things, I believe there are a batch of PFC changes that
> will make sense to take through the arm-soc tree due to dependencies.
> 
It doesn't particularly matter to me where the updates go, so long as
everyone is kept aware of what changes are being made before they're
silently merged somewhere.

Stuff like this falls more in to the obvious bugfix category, and is
probably fine for arm-soc, given that it's largely SoC-specific. I would
prefer if the sh-pfc core stuff remain independent of arm-soc however,
but it's not terribly important whether I merge it or Linus does.

Re: [PATCH] sh-pfc: r8a7779: fixup INDTx address

[Kuninori Morimoto]

Hi Simon

> Morimoto-san,
> 
> could you comment on how critical this change is.
> Does anything in 3.9-rc1 depend on it?

Now, R-Car H1 board is only Marzen,
and it doesn't have GPIO input/output device (like gpio-key)
at this point
I guess, it is not critical in v3.9.

Best regards
---
Kuninori Morimoto

Re: [PATCH] sh-pfc: r8a7779: fixup INDTx address

[Simon Horman]

On Mon, Mar 04, 2013 at 08:14:34PM -0800, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > Morimoto-san,
> > 
> > could you comment on how critical this change is.
> > Does anything in 3.9-rc1 depend on it?
> 
> Now, R-Car H1 board is only Marzen,
> and it doesn't have GPIO input/output device (like gpio-key)
> at this point
> I guess, it is not critical in v3.9.

Thanks, it sounds like it can just be queued up for the next release.

Re: [PATCH] sh-pfc: r8a7779: fixup INDTx address

[phil.edworthy]

Hi Morimoto-san, Simon,

> Subject: [PATCH] sh-pfc: r8a7779: fixup INDTx address
> Sent by: linux-sh-owner@vger.kernel.org
> 
> INDTn address is 0xFFC4x000C. 0xFFC4x0008 is OUTDTn
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  drivers/pinctrl/sh-pfc/pfc-r8a7779.c |   14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7779.c b/drivers/pinctrl/
> sh-pfc/pfc-r8a7779.c
> index 13feaa0..cb97fb6 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7779.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7779.c
> @@ -2585,13 +2585,13 @@ static struct pinmux_cfg_reg 
pinmux_config_regs[] = {
>  };
> 
>  static struct pinmux_data_reg pinmux_data_regs[] = {
> -   { PINMUX_DATA_REG("INDT0", 0xffc40008, 32) { GP_INDT(0) } },
> -   { PINMUX_DATA_REG("INDT1", 0xffc41008, 32) { GP_INDT(1) } },
> -   { PINMUX_DATA_REG("INDT2", 0xffc42008, 32) { GP_INDT(2) } },
> -   { PINMUX_DATA_REG("INDT3", 0xffc43008, 32) { GP_INDT(3) } },
> -   { PINMUX_DATA_REG("INDT4", 0xffc44008, 32) { GP_INDT(4) } },
> -   { PINMUX_DATA_REG("INDT5", 0xffc45008, 32) { GP_INDT(5) } },
> -   { PINMUX_DATA_REG("INDT6", 0xffc46008, 32) {
> +   { PINMUX_DATA_REG("INDT0", 0xffc4000c, 32) { GP_INDT(0) } },
> +   { PINMUX_DATA_REG("INDT1", 0xffc4100c, 32) { GP_INDT(1) } },
> +   { PINMUX_DATA_REG("INDT2", 0xffc4200c, 32) { GP_INDT(2) } },
> +   { PINMUX_DATA_REG("INDT3", 0xffc4300c, 32) { GP_INDT(3) } },
> +   { PINMUX_DATA_REG("INDT4", 0xffc4400c, 32) { GP_INDT(4) } },
> +   { PINMUX_DATA_REG("INDT5", 0xffc4500c, 32) { GP_INDT(5) } },
> +   { PINMUX_DATA_REG("INDT6", 0xffc4600c, 32) {
>        0, 0, 0, 0, 0, 0, 0, 0,   0, 0, 0, 0, 0, 0, 0, 0,
>        0, 0, 0, 0, 0, 0, 0, GP_6_8_DATA,
>        GP_6_7_DATA, GP_6_6_DATA, GP_6_5_DATA, GP_6_4_DATA,

Hmm, this isn't what you want. I sent a similar patch on this a while back 
before realising that the problem is that the pfc driver doesn't support 
separate input and output data registers, only one reg for both input and 
output. With the code as it is, the gpio pins act as outputs. Changing the 
addresses as per this patch means the gpios act now as inputs.

Thanks
Phil

Re: [PATCH] sh-pfc: r8a7779: fixup INDTx address

[Laurent Pinchart]

Hi Simon,

On Tuesday 05 March 2013 12:18:52 Simon Horman wrote:
> [ Cc: Linus Walleij ]
> 
> Hi Linus, Hi Paul, Hi Laurent,
> 
> I'm a little unsure how everyone would like to handle
> drivers/pinctrl/sh-pfc/ updates.
> 
> I'm happy to create a pfc branch and send pull requests to Linus.
> Or for Paul to handle things. Or for Laurent to handle things.
> Or for other suggestions.

I would take this kind of patches in my tree, except that in this case the 
patch isn't correct, as pointed out by Phil in his reply.

> To confuse things, I believe there are a batch of PFC changes that
> will make sense to take through the arm-soc tree due to dependencies.

I'm working on the next batch, I will submit it today or tomorrow.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] sh-pfc: r8a7779: fixup INDTx address

[Kuninori Morimoto]

Hi Phil

Thank you for your reply

> >  static struct pinmux_data_reg pinmux_data_regs[] = {
> > -   { PINMUX_DATA_REG("INDT0", 0xffc40008, 32) { GP_INDT(0) } },
> > -   { PINMUX_DATA_REG("INDT1", 0xffc41008, 32) { GP_INDT(1) } },
> > -   { PINMUX_DATA_REG("INDT2", 0xffc42008, 32) { GP_INDT(2) } },
> > -   { PINMUX_DATA_REG("INDT3", 0xffc43008, 32) { GP_INDT(3) } },
> > -   { PINMUX_DATA_REG("INDT4", 0xffc44008, 32) { GP_INDT(4) } },
> > -   { PINMUX_DATA_REG("INDT5", 0xffc45008, 32) { GP_INDT(5) } },
> > -   { PINMUX_DATA_REG("INDT6", 0xffc46008, 32) {
> > +   { PINMUX_DATA_REG("INDT0", 0xffc4000c, 32) { GP_INDT(0) } },
> > +   { PINMUX_DATA_REG("INDT1", 0xffc4100c, 32) { GP_INDT(1) } },
> > +   { PINMUX_DATA_REG("INDT2", 0xffc4200c, 32) { GP_INDT(2) } },
> > +   { PINMUX_DATA_REG("INDT3", 0xffc4300c, 32) { GP_INDT(3) } },
> > +   { PINMUX_DATA_REG("INDT4", 0xffc4400c, 32) { GP_INDT(4) } },
> > +   { PINMUX_DATA_REG("INDT5", 0xffc4500c, 32) { GP_INDT(5) } },
> > +   { PINMUX_DATA_REG("INDT6", 0xffc4600c, 32) {
> >        0, 0, 0, 0, 0, 0, 0, 0,   0, 0, 0, 0, 0, 0, 0, 0,
> >        0, 0, 0, 0, 0, 0, 0, GP_6_8_DATA,
> >        GP_6_7_DATA, GP_6_6_DATA, GP_6_5_DATA, GP_6_4_DATA,
> 
> Hmm, this isn't what you want. I sent a similar patch on this a while back 
> before realising that the problem is that the pfc driver doesn't support 
> separate input and output data registers, only one reg for both input and 
> output. With the code as it is, the gpio pins act as outputs. Changing the 
> addresses as per this patch means the gpios act now as inputs.

This patch ?
[sh: pfc: Add ability to use separate read & write GPIO data regs]

I need more information about this, because my previous R-Car M1 GPIO is
using INPUT address like above

 - Is your patch (above patch ?) applied to Simon's branch ?
 - Is some SoC (= H1 ?) using it ?

I need sample code

Best regards
---
Kuninori Morimoto

Re: [PATCH] sh-pfc: r8a7779: fixup INDTx address

[Simon Horman]

On Tue, Mar 05, 2013 at 04:51:28PM -0800, Kuninori Morimoto wrote:
> 
> Hi Phil
> 
> Thank you for your reply
> 
> > >  static struct pinmux_data_reg pinmux_data_regs[] = {
> > > -   { PINMUX_DATA_REG("INDT0", 0xffc40008, 32) { GP_INDT(0) } },
> > > -   { PINMUX_DATA_REG("INDT1", 0xffc41008, 32) { GP_INDT(1) } },
> > > -   { PINMUX_DATA_REG("INDT2", 0xffc42008, 32) { GP_INDT(2) } },
> > > -   { PINMUX_DATA_REG("INDT3", 0xffc43008, 32) { GP_INDT(3) } },
> > > -   { PINMUX_DATA_REG("INDT4", 0xffc44008, 32) { GP_INDT(4) } },
> > > -   { PINMUX_DATA_REG("INDT5", 0xffc45008, 32) { GP_INDT(5) } },
> > > -   { PINMUX_DATA_REG("INDT6", 0xffc46008, 32) {
> > > +   { PINMUX_DATA_REG("INDT0", 0xffc4000c, 32) { GP_INDT(0) } },
> > > +   { PINMUX_DATA_REG("INDT1", 0xffc4100c, 32) { GP_INDT(1) } },
> > > +   { PINMUX_DATA_REG("INDT2", 0xffc4200c, 32) { GP_INDT(2) } },
> > > +   { PINMUX_DATA_REG("INDT3", 0xffc4300c, 32) { GP_INDT(3) } },
> > > +   { PINMUX_DATA_REG("INDT4", 0xffc4400c, 32) { GP_INDT(4) } },
> > > +   { PINMUX_DATA_REG("INDT5", 0xffc4500c, 32) { GP_INDT(5) } },
> > > +   { PINMUX_DATA_REG("INDT6", 0xffc4600c, 32) {
> > >        0, 0, 0, 0, 0, 0, 0, 0,   0, 0, 0, 0, 0, 0, 0, 0,
> > >        0, 0, 0, 0, 0, 0, 0, GP_6_8_DATA,
> > >        GP_6_7_DATA, GP_6_6_DATA, GP_6_5_DATA, GP_6_4_DATA,
> > 
> > Hmm, this isn't what you want. I sent a similar patch on this a while back 
> > before realising that the problem is that the pfc driver doesn't support 
> > separate input and output data registers, only one reg for both input and 
> > output. With the code as it is, the gpio pins act as outputs. Changing the 
> > addresses as per this patch means the gpios act now as inputs.
> 
> This patch ?
> [sh: pfc: Add ability to use separate read & write GPIO data regs]
> 
> I need more information about this, because my previous R-Car M1 GPIO is
> using INPUT address like above
> 
>  - Is your patch (above patch ?) applied to Simon's branch ?

No, it is not in my tree.
I would be happy to add it if Laurent is happy with it.
Or for him to include it in his forthcoming pinctrl patch set(s).

>  - Is some SoC (= H1 ?) using it ?
> 
> I need sample code

Re: [PATCH] sh-pfc: r8a7779: fixup INDTx address

[Simon Horman]

On Wed, Mar 06, 2013 at 01:39:58AM +0100, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Tuesday 05 March 2013 12:18:52 Simon Horman wrote:
> > [ Cc: Linus Walleij ]
> > 
> > Hi Linus, Hi Paul, Hi Laurent,
> > 
> > I'm a little unsure how everyone would like to handle
> > drivers/pinctrl/sh-pfc/ updates.
> > 
> > I'm happy to create a pfc branch and send pull requests to Linus.
> > Or for Paul to handle things. Or for Laurent to handle things.
> > Or for other suggestions.
> 
> I would take this kind of patches in my tree, except that in this case the 
> patch isn't correct, as pointed out by Phil in his reply.

Thanks, got it.

> > To confuse things, I believe there are a batch of PFC changes that
> > will make sense to take through the arm-soc tree due to dependencies.
> 
> I'm working on the next batch, I will submit it today or tomorrow.

Great.

Re: [PATCH] sh-pfc: r8a7779: fixup INDTx address

[phil.edworthy]

Hi Simon & Morimoto-san,

> > > >  static struct pinmux_data_reg pinmux_data_regs[] = {
> > > > -   { PINMUX_DATA_REG("INDT0", 0xffc40008, 32) { GP_INDT(0) } },
> > > > -   { PINMUX_DATA_REG("INDT1", 0xffc41008, 32) { GP_INDT(1) } },
> > > > -   { PINMUX_DATA_REG("INDT2", 0xffc42008, 32) { GP_INDT(2) } },
> > > > -   { PINMUX_DATA_REG("INDT3", 0xffc43008, 32) { GP_INDT(3) } },
> > > > -   { PINMUX_DATA_REG("INDT4", 0xffc44008, 32) { GP_INDT(4) } },
> > > > -   { PINMUX_DATA_REG("INDT5", 0xffc45008, 32) { GP_INDT(5) } },
> > > > -   { PINMUX_DATA_REG("INDT6", 0xffc46008, 32) {
> > > > +   { PINMUX_DATA_REG("INDT0", 0xffc4000c, 32) { GP_INDT(0) } },
> > > > +   { PINMUX_DATA_REG("INDT1", 0xffc4100c, 32) { GP_INDT(1) } },
> > > > +   { PINMUX_DATA_REG("INDT2", 0xffc4200c, 32) { GP_INDT(2) } },
> > > > +   { PINMUX_DATA_REG("INDT3", 0xffc4300c, 32) { GP_INDT(3) } },
> > > > +   { PINMUX_DATA_REG("INDT4", 0xffc4400c, 32) { GP_INDT(4) } },
> > > > +   { PINMUX_DATA_REG("INDT5", 0xffc4500c, 32) { GP_INDT(5) } },
> > > > +   { PINMUX_DATA_REG("INDT6", 0xffc4600c, 32) {
> > > >        0, 0, 0, 0, 0, 0, 0, 0,   0, 0, 0, 0, 0, 0, 0, 0,
> > > >        0, 0, 0, 0, 0, 0, 0, GP_6_8_DATA,
> > > >        GP_6_7_DATA, GP_6_6_DATA, GP_6_5_DATA, GP_6_4_DATA,
> > > 
> > > Hmm, this isn't what you want. I sent a similar patch on this a 
> while back 
> > > before realising that the problem is that the pfc driver doesn't 
support 
> > > separate input and output data registers, only one reg for both 
input and 
> > > output. With the code as it is, the gpio pins act as outputs. 
> Changing the 
> > > addresses as per this patch means the gpios act now as inputs.
> > 
> > This patch ?
> > [sh: pfc: Add ability to use separate read & write GPIO data regs]
Yes, this is the patch I was referring to,

> > I need more information about this, because my previous R-Car M1 GPIO 
is
> > using INPUT address like above
> > 
> >  - Is your patch (above patch ?) applied to Simon's branch ?
> 
> No, it is not in my tree.
> I would be happy to add it if Laurent is happy with it.
> Or for him to include it in his forthcoming pinctrl patch set(s).
Paul gave the patch a quick review & pointed out that I really needed to 
use clearer nomenclature. He also suggested that it might be better to use 
a single mapping for both INDT and OUTDT registers in the pfc driver. 
Whilst the latter is easy enough to fix if the order of INDT and OUTDT are 
always the same, I don't know if this is the case for all/upcoming 
devices.

Anyhow, I don't think that Morimoto-san's "sh-pfc: r8a7779: fixup INDTx 
address" patch should be used as it changes the functionality (not what it 
says in the description) and doesn't fix the underlying issue.

Thanks
Phil

Re: [PATCH] sh-pfc: r8a7779: fixup INDTx address

[Simon Horman]

On Wed, Mar 06, 2013 at 07:18:39AM +0000, phil.edworthy@renesas.com wrote:
> Hi Simon & Morimoto-san,
> 
> > > > >  static struct pinmux_data_reg pinmux_data_regs[] = {
> > > > > -   { PINMUX_DATA_REG("INDT0", 0xffc40008, 32) { GP_INDT(0) } },
> > > > > -   { PINMUX_DATA_REG("INDT1", 0xffc41008, 32) { GP_INDT(1) } },
> > > > > -   { PINMUX_DATA_REG("INDT2", 0xffc42008, 32) { GP_INDT(2) } },
> > > > > -   { PINMUX_DATA_REG("INDT3", 0xffc43008, 32) { GP_INDT(3) } },
> > > > > -   { PINMUX_DATA_REG("INDT4", 0xffc44008, 32) { GP_INDT(4) } },
> > > > > -   { PINMUX_DATA_REG("INDT5", 0xffc45008, 32) { GP_INDT(5) } },
> > > > > -   { PINMUX_DATA_REG("INDT6", 0xffc46008, 32) {
> > > > > +   { PINMUX_DATA_REG("INDT0", 0xffc4000c, 32) { GP_INDT(0) } },
> > > > > +   { PINMUX_DATA_REG("INDT1", 0xffc4100c, 32) { GP_INDT(1) } },
> > > > > +   { PINMUX_DATA_REG("INDT2", 0xffc4200c, 32) { GP_INDT(2) } },
> > > > > +   { PINMUX_DATA_REG("INDT3", 0xffc4300c, 32) { GP_INDT(3) } },
> > > > > +   { PINMUX_DATA_REG("INDT4", 0xffc4400c, 32) { GP_INDT(4) } },
> > > > > +   { PINMUX_DATA_REG("INDT5", 0xffc4500c, 32) { GP_INDT(5) } },
> > > > > +   { PINMUX_DATA_REG("INDT6", 0xffc4600c, 32) {
> > > > >        0, 0, 0, 0, 0, 0, 0, 0,   0, 0, 0, 0, 0, 0, 0, 0,
> > > > >        0, 0, 0, 0, 0, 0, 0, GP_6_8_DATA,
> > > > >        GP_6_7_DATA, GP_6_6_DATA, GP_6_5_DATA, GP_6_4_DATA,
> > > > 
> > > > Hmm, this isn't what you want. I sent a similar patch on this a 
> > while back 
> > > > before realising that the problem is that the pfc driver doesn't 
> support 
> > > > separate input and output data registers, only one reg for both 
> input and 
> > > > output. With the code as it is, the gpio pins act as outputs. 
> > Changing the 
> > > > addresses as per this patch means the gpios act now as inputs.
> > > 
> > > This patch ?
> > > [sh: pfc: Add ability to use separate read & write GPIO data regs]
> Yes, this is the patch I was referring to,
> 
> > > I need more information about this, because my previous R-Car M1 GPIO 
> is
> > > using INPUT address like above
> > > 
> > >  - Is your patch (above patch ?) applied to Simon's branch ?
> > 
> > No, it is not in my tree.
> > I would be happy to add it if Laurent is happy with it.
> > Or for him to include it in his forthcoming pinctrl patch set(s).
> Paul gave the patch a quick review & pointed out that I really needed to 
> use clearer nomenclature. He also suggested that it might be better to use 
> a single mapping for both INDT and OUTDT registers in the pfc driver. 
> Whilst the latter is easy enough to fix if the order of INDT and OUTDT are 
> always the same, I don't know if this is the case for all/upcoming 
> devices.

Thanks, I understand.

Perhaps Morimoto-san could revise the patch?

> Anyhow, I don't think that Morimoto-san's "sh-pfc: r8a7779: fixup INDTx 
> address" patch should be used as it changes the functionality (not what it 
> says in the description) and doesn't fix the underlying issue.

Agreed.

Re: [PATCH] sh-pfc: r8a7779: fixup INDTx address

[Magnus Damm]

On Wed, Mar 6, 2013 at 2:51 AM,  <phil.edworthy@renesas.com> wrote:
> Hi Morimoto-san, Simon,
>
>> Subject: [PATCH] sh-pfc: r8a7779: fixup INDTx address
>> Sent by: linux-sh-owner@vger.kernel.org
>>
>> INDTn address is 0xFFC4x000C. 0xFFC4x0008 is OUTDTn
>>
>> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>> ---
>>  drivers/pinctrl/sh-pfc/pfc-r8a7779.c |   14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7779.c b/drivers/pinctrl/
>> sh-pfc/pfc-r8a7779.c
>> index 13feaa0..cb97fb6 100644
>> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7779.c
>> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7779.c
>> @@ -2585,13 +2585,13 @@ static struct pinmux_cfg_reg
> pinmux_config_regs[] = {
>>  };
>>
>>  static struct pinmux_data_reg pinmux_data_regs[] = {
>> -   { PINMUX_DATA_REG("INDT0", 0xffc40008, 32) { GP_INDT(0) } },
>> -   { PINMUX_DATA_REG("INDT1", 0xffc41008, 32) { GP_INDT(1) } },
>> -   { PINMUX_DATA_REG("INDT2", 0xffc42008, 32) { GP_INDT(2) } },
>> -   { PINMUX_DATA_REG("INDT3", 0xffc43008, 32) { GP_INDT(3) } },
>> -   { PINMUX_DATA_REG("INDT4", 0xffc44008, 32) { GP_INDT(4) } },
>> -   { PINMUX_DATA_REG("INDT5", 0xffc45008, 32) { GP_INDT(5) } },
>> -   { PINMUX_DATA_REG("INDT6", 0xffc46008, 32) {
>> +   { PINMUX_DATA_REG("INDT0", 0xffc4000c, 32) { GP_INDT(0) } },
>> +   { PINMUX_DATA_REG("INDT1", 0xffc4100c, 32) { GP_INDT(1) } },
>> +   { PINMUX_DATA_REG("INDT2", 0xffc4200c, 32) { GP_INDT(2) } },
>> +   { PINMUX_DATA_REG("INDT3", 0xffc4300c, 32) { GP_INDT(3) } },
>> +   { PINMUX_DATA_REG("INDT4", 0xffc4400c, 32) { GP_INDT(4) } },
>> +   { PINMUX_DATA_REG("INDT5", 0xffc4500c, 32) { GP_INDT(5) } },
>> +   { PINMUX_DATA_REG("INDT6", 0xffc4600c, 32) {
>>        0, 0, 0, 0, 0, 0, 0, 0,   0, 0, 0, 0, 0, 0, 0, 0,
>>        0, 0, 0, 0, 0, 0, 0, GP_6_8_DATA,
>>        GP_6_7_DATA, GP_6_6_DATA, GP_6_5_DATA, GP_6_4_DATA,
>
> Hmm, this isn't what you want. I sent a similar patch on this a while back
> before realising that the problem is that the pfc driver doesn't support
> separate input and output data registers, only one reg for both input and
> output. With the code as it is, the gpio pins act as outputs. Changing the
> addresses as per this patch means the gpios act now as inputs.

Hi Phil,

Thanks for guiding people in the right direction. GPIOS on r8a7779 is
a bit tricky in general. Both IN and OUT registers as well as IRQ
demux support.

From my point of view, switching to the recently posted gpio-rcar.c
driver should take care of all these issues.
[PATCH] gpio: Renesas R-Car GPIO driver

We do however need to work on how to make GPIO and PFC coexist.

Cheers,

/ magnus

Re: [PATCH] sh-pfc: r8a7779: fixup INDTx address

[Kuninori Morimoto]

Hi Simon, Phil

> > > No, it is not in my tree.
> > > I would be happy to add it if Laurent is happy with it.
> > > Or for him to include it in his forthcoming pinctrl patch set(s).
> > Paul gave the patch a quick review & pointed out that I really needed to 
> > use clearer nomenclature. He also suggested that it might be better to use 
> > a single mapping for both INDT and OUTDT registers in the pfc driver. 
> > Whilst the latter is easy enough to fix if the order of INDT and OUTDT are 
> > always the same, I don't know if this is the case for all/upcoming 
> > devices.
> 
> Thanks, I understand.
> 
> Perhaps Morimoto-san could revise the patch?

I see.
I thought that it was just bug, but I'm busy for now...
sorry

> > Anyhow, I don't think that Morimoto-san's "sh-pfc: r8a7779: fixup INDTx 
> > address" patch should be used as it changes the functionality (not what it 
> > says in the description) and doesn't fix the underlying issue.
> 
> Agreed.

OK
But, this means that I should modify R-Car M1's GPIO INDTx address...

Best regards
---
Kuninori Morimoto

Re: [PATCH] sh-pfc: r8a7779: fixup INDTx address

[phil.edworthy]

Hi Magnus,

> From: Magnus Damm <magnus.damm@gmail.com>
> To: phil.edworthy@renesas.com, 
> Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>, Simon 
> <horms@verge.net.au>, Kuninori Morimoto 
> <kuninori.morimoto.gx@gmail.com>, Laurent 
> <laurent.pinchart@ideasonboard.com>, Paul Mundt <lethal@linux-
> sh.org>, Linux-SH <linux-sh@vger.kernel.org>, 
linux-sh-owner@vger.kernel.org
> Date: 06/03/2013 07:53
> Subject: Re: [PATCH] sh-pfc: r8a7779: fixup INDTx address
> 
> On Wed, Mar 6, 2013 at 2:51 AM,  <phil.edworthy@renesas.com> wrote:
> > Hi Morimoto-san, Simon,
> >
> >> Subject: [PATCH] sh-pfc: r8a7779: fixup INDTx address
> >> Sent by: linux-sh-owner@vger.kernel.org
> >>
> >> INDTn address is 0xFFC4x000C. 0xFFC4x0008 is OUTDTn
> >>
> >> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> >> ---
> >>  drivers/pinctrl/sh-pfc/pfc-r8a7779.c |   14 +++++++-------
> >>  1 file changed, 7 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7779.c b/drivers/pinctrl/
> >> sh-pfc/pfc-r8a7779.c
> >> index 13feaa0..cb97fb6 100644
> >> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7779.c
> >> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7779.c
> >> @@ -2585,13 +2585,13 @@ static struct pinmux_cfg_reg
> > pinmux_config_regs[] = {
> >>  };
> >>
> >>  static struct pinmux_data_reg pinmux_data_regs[] = {
> >> -   { PINMUX_DATA_REG("INDT0", 0xffc40008, 32) { GP_INDT(0) } },
> >> -   { PINMUX_DATA_REG("INDT1", 0xffc41008, 32) { GP_INDT(1) } },
> >> -   { PINMUX_DATA_REG("INDT2", 0xffc42008, 32) { GP_INDT(2) } },
> >> -   { PINMUX_DATA_REG("INDT3", 0xffc43008, 32) { GP_INDT(3) } },
> >> -   { PINMUX_DATA_REG("INDT4", 0xffc44008, 32) { GP_INDT(4) } },
> >> -   { PINMUX_DATA_REG("INDT5", 0xffc45008, 32) { GP_INDT(5) } },
> >> -   { PINMUX_DATA_REG("INDT6", 0xffc46008, 32) {
> >> +   { PINMUX_DATA_REG("INDT0", 0xffc4000c, 32) { GP_INDT(0) } },
> >> +   { PINMUX_DATA_REG("INDT1", 0xffc4100c, 32) { GP_INDT(1) } },
> >> +   { PINMUX_DATA_REG("INDT2", 0xffc4200c, 32) { GP_INDT(2) } },
> >> +   { PINMUX_DATA_REG("INDT3", 0xffc4300c, 32) { GP_INDT(3) } },
> >> +   { PINMUX_DATA_REG("INDT4", 0xffc4400c, 32) { GP_INDT(4) } },
> >> +   { PINMUX_DATA_REG("INDT5", 0xffc4500c, 32) { GP_INDT(5) } },
> >> +   { PINMUX_DATA_REG("INDT6", 0xffc4600c, 32) {
> >>        0, 0, 0, 0, 0, 0, 0, 0,   0, 0, 0, 0, 0, 0, 0, 0,
> >>        0, 0, 0, 0, 0, 0, 0, GP_6_8_DATA,
> >>        GP_6_7_DATA, GP_6_6_DATA, GP_6_5_DATA, GP_6_4_DATA,
> >
> > Hmm, this isn't what you want. I sent a similar patch on this a while 
back
> > before realising that the problem is that the pfc driver doesn't 
support
> > separate input and output data registers, only one reg for both input 
and
> > output. With the code as it is, the gpio pins act as outputs. Changing 
the
> > addresses as per this patch means the gpios act now as inputs.
> 
> Hi Phil,
> 
> Thanks for guiding people in the right direction. GPIOS on r8a7779 is
> a bit tricky in general. Both IN and OUT registers as well as IRQ
> demux support.
> 
> From my point of view, switching to the recently posted gpio-rcar.c
> driver should take care of all these issues.
> [PATCH] gpio: Renesas R-Car GPIO driver
I hadn't seen that, thanks for pointing it out.

> We do however need to work on how to make GPIO and PFC coexist.
Yes, that is certainly an issue...

Thanks
Phil

Re: [PATCH] sh-pfc: r8a7779: fixup INDTx address

[Magnus Damm]

Hi Phil

On Wed, Mar 6, 2013 at 5:50 PM,  <phil.edworthy@renesas.com> wrote:
>> > Hmm, this isn't what you want. I sent a similar patch on this a while
> back
>> > before realising that the problem is that the pfc driver doesn't
> support
>> > separate input and output data registers, only one reg for both input
> and
>> > output. With the code as it is, the gpio pins act as outputs. Changing
> the
>> > addresses as per this patch means the gpios act now as inputs.
>>
>> Hi Phil,
>>
>> Thanks for guiding people in the right direction. GPIOS on r8a7779 is
>> a bit tricky in general. Both IN and OUT registers as well as IRQ
>> demux support.
>>
>> From my point of view, switching to the recently posted gpio-rcar.c
>> driver should take care of all these issues.
>> [PATCH] gpio: Renesas R-Car GPIO driver
> I hadn't seen that, thanks for pointing it out.
>
>> We do however need to work on how to make GPIO and PFC coexist.
> Yes, that is certainly an issue...

Yeah, (un?)fortunately it's something we need on a wide range of SoCs
including EMEV2, R-Car H1, R-Car M1A and a bunch of newer SoCs as
well.

Thanks,

/ magnus