diff mbox

pinctrl: rockchip: fix offset of mux registers for rk3188

Message ID 1395700561-3793-1-git-send-email-b.galvani@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Beniamino Galvani March 24, 2014, 10:36 p.m. UTC
The correct value of .mux_offset for rk3188 seems to be 0x60
instead of 0x68.

Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>
---
 drivers/pinctrl/pinctrl-rockchip.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Heiko Stübner March 24, 2014, 11:14 p.m. UTC | #1
Am Montag, 24. März 2014, 23:36:01 schrieb Beniamino Galvani:
> The correct value of .mux_offset for rk3188 seems to be 0x60
> instead of 0x68.

Executive summary: the offset-change itself is correct, therefore

Reviewed-by: Heiko Stuebner <heiko@sntech.de>



That is what one gets when the only source is a vendor tree.
I've looked it up again, and it seems you're right with the offset, but there 
seems to be more to it ;-)

GPIO0 only has the second two IOMUX registers:
- GRF_GPIO0C_IOMUX at 0x68
- GRF_GPIO0D_IOMUX at 0x6c
which I guess is where my mistake comes from.

It looks like there does no iomux register exist at all for the first 16 pins.

In any case, the current number is wrong, and the 0x60 offset is the correct 
one, but I guess we need to determine what the affected pins do - do they 
always have a gpio mux or such?


Thanks for catching the mistake.

Heiko

> Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>
> ---
>  drivers/pinctrl/pinctrl-rockchip.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c
> b/drivers/pinctrl/pinctrl-rockchip.c index 46dddc1..23e8812 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -1534,7 +1534,7 @@ static struct rockchip_pin_ctrl rk3188_pin_ctrl = {
>  		.nr_banks		= ARRAY_SIZE(rk3188_pin_banks),
>  		.label			= "RK3188-GPIO",
>  		.type			= RK3188,
> -		.mux_offset		= 0x68,
> +		.mux_offset		= 0x60,
>  		.pull_calc_reg		= rk3188_calc_pull_reg_and_bit,
>  };
Beniamino Galvani March 25, 2014, 7:43 p.m. UTC | #2
On Tue, Mar 25, 2014 at 12:14:42AM +0100, Heiko Stübner wrote:
> Am Montag, 24. März 2014, 23:36:01 schrieb Beniamino Galvani:
> > The correct value of .mux_offset for rk3188 seems to be 0x60
> > instead of 0x68.
> 
> Executive summary: the offset-change itself is correct, therefore
> 
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> 
> That is what one gets when the only source is a vendor tree.
> I've looked it up again, and it seems you're right with the offset, but there 
> seems to be more to it ;-)
> 
> GPIO0 only has the second two IOMUX registers:
> - GRF_GPIO0C_IOMUX at 0x68
> - GRF_GPIO0D_IOMUX at 0x6c
> which I guess is where my mistake comes from.
> 
> It looks like there does no iomux register exist at all for the first 16 pins.
> 
> In any case, the current number is wrong, and the 0x60 offset is the correct 
> one, but I guess we need to determine what the affected pins do - do they 
> always have a gpio mux or such?

On radxa rock schematic pins GPIO0A* and GPIO0B* are labeled only as
gpios, without alternate functions like other pins; my guess is that
on rk3188 they can only act as gpios and so mux registers are not
needed for them.

Beniamino

> 
> Thanks for catching the mistake.
> 
> Heiko
> 
> > Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>
> > ---
> >  drivers/pinctrl/pinctrl-rockchip.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pinctrl/pinctrl-rockchip.c
> > b/drivers/pinctrl/pinctrl-rockchip.c index 46dddc1..23e8812 100644
> > --- a/drivers/pinctrl/pinctrl-rockchip.c
> > +++ b/drivers/pinctrl/pinctrl-rockchip.c
> > @@ -1534,7 +1534,7 @@ static struct rockchip_pin_ctrl rk3188_pin_ctrl = {
> >  		.nr_banks		= ARRAY_SIZE(rk3188_pin_banks),
> >  		.label			= "RK3188-GPIO",
> >  		.type			= RK3188,
> > -		.mux_offset		= 0x68,
> > +		.mux_offset		= 0x60,
> >  		.pull_calc_reg		= rk3188_calc_pull_reg_and_bit,
> >  };
>
Heiko Stübner March 25, 2014, 11:56 p.m. UTC | #3
Am Dienstag, 25. März 2014, 20:43:59 schrieb Beniamino Galvani:
> On Tue, Mar 25, 2014 at 12:14:42AM +0100, Heiko Stübner wrote:
> > GPIO0 only has the second two IOMUX registers:
> > - GRF_GPIO0C_IOMUX at 0x68
> > - GRF_GPIO0D_IOMUX at 0x6c
> > which I guess is where my mistake comes from.

[...] 

> On radxa rock schematic pins GPIO0A* and GPIO0B* are labeled only as
> gpios, without alternate functions like other pins; my guess is that
> on rk3188 they can only act as gpios and so mux registers are not
> needed for them.

That was my guess too - especially as the registers are also missing.

Therefore I put together the following two patches to go on top of
your patch and also make rockchip_set_mux honor this situation.


Heiko Stuebner (2):
  pinctrl: rockchip: add return value to rockchip_set_mux
  pinctrl: rockchip: handle first half of rk3188-bank0 correctly

 drivers/pinctrl/pinctrl-rockchip.c | 46 ++++++++++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 7 deletions(-)
Linus Walleij April 3, 2014, 2:10 p.m. UTC | #4
On Mon, Mar 24, 2014 at 11:36 PM, Beniamino Galvani <b.galvani@gmail.com> wrote:

> The correct value of .mux_offset for rk3188 seems to be 0x60
> instead of 0x68.
>
> Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>

Patch applied to fixes with Heiko's Review tag and also
copied some info from the conversation into the commit message.

Yours,
Linus Walleij
Linus Walleij April 3, 2014, 2:13 p.m. UTC | #5
On Wed, Mar 26, 2014 at 12:56 AM, Heiko Stübner <heiko@sntech.de> wrote:

> Therefore I put together the following two patches to go on top of
> your patch and also make rockchip_set_mux honor this situation.

Both patches applied for fixes.

Yours,
Linus Walleij
Heiko Stübner April 3, 2014, 2:20 p.m. UTC | #6
Am Donnerstag, 3. April 2014, 16:10:46 schrieb Linus Walleij:
> On Mon, Mar 24, 2014 at 11:36 PM, Beniamino Galvani <b.galvani@gmail.com> 
wrote:
> > The correct value of .mux_offset for rk3188 seems to be 0x60
> > instead of 0x68.
> > 
> > Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>
> 
> Patch applied to fixes with Heiko's Review tag and also
> copied some info from the conversation into the commit message.

small protest note

"Heikki adds:" - sounds somehow finish or swedish, but who is this guy? ;-)


But it doesn't really matter, so no need to roll it back

Heiko
Linus Walleij April 3, 2014, 3:27 p.m. UTC | #7
On Thu, Apr 3, 2014 at 4:20 PM, Heiko Stübner <heiko@sntech.de> wrote:

> small protest note
>
> "Heikki adds:" - sounds somehow finish or swedish, but who is this guy? ;-)

Haha sorry Heiko, I'll fix.

Yours,
Linus Walleij
diff mbox

Patch

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 46dddc1..23e8812 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -1534,7 +1534,7 @@  static struct rockchip_pin_ctrl rk3188_pin_ctrl = {
 		.nr_banks		= ARRAY_SIZE(rk3188_pin_banks),
 		.label			= "RK3188-GPIO",
 		.type			= RK3188,
-		.mux_offset		= 0x68,
+		.mux_offset		= 0x60,
 		.pull_calc_reg		= rk3188_calc_pull_reg_and_bit,
 };