Message ID | 1401272177-16107-2-git-send-email-hs@denx.de |
---|---|
State | Superseded |
Delegated to: | Stefano Babic |
Headers | show |
Hi Heiko, On 28/05/2014 12:16, Heiko Schocher wrote: > add mxc_iomux_set_gpr_register fucntion > > Signed-off-by: Heiko Schocher <hs@denx.de> > Cc: Stefano Babic <sbabic@denx.de> > --- > arch/arm/imx-common/iomux-v3.c | 15 +++++++++++++++ > arch/arm/include/asm/imx-common/iomux-v3.h | 2 ++ > 2 files changed, 17 insertions(+) > > diff --git a/arch/arm/imx-common/iomux-v3.c b/arch/arm/imx-common/iomux-v3.c > index 6e46ea8..8739b4c 100644 > --- a/arch/arm/imx-common/iomux-v3.c > +++ b/arch/arm/imx-common/iomux-v3.c > @@ -63,3 +63,18 @@ void imx_iomux_v3_setup_multiple_pads(iomux_v3_cfg_t const *pad_list, > for (i = 0; i < count; i++) > imx_iomux_v3_setup_pad(*p++); > } > + > +void mxc_iomux_set_gpr_register(int group, int start_bit, int num_bits, > + int value) > +{ > + int i = 0; > + u32 reg; > + reg = __raw_readl(base + group * 4); > + while (num_bits) { > + reg &= ~(1<<(start_bit + i)); > + i++; > + num_bits--; > + } > + reg |= (value << start_bit); > + __raw_writel(reg, base + group * 4); > +} It is not clear to me why we need to add this or better, I want to avoid having twice quite the same code doing the same ;-). We have already a field for gpr: struct iomuxc { u32 gpr[14]; u32 omux[5]; /* mux and pad registers */ }; + reg = __raw_readl(base + group * 4); this could not work when Freescale introduce a new variant with a slight different layout. Thanks to Fabio, we know this is already reality: http://patchwork.ozlabs.org/patch/368336/ Your code *does* not work with the solox. Generally, we access register and we use setbits_ and clrsetbits_ to modify single bits. What is the advantage here ? > diff --git a/arch/arm/include/asm/imx-common/iomux-v3.h b/arch/arm/include/asm/imx-common/iomux-v3.h > index cca920b..4c83617 100644 > --- a/arch/arm/include/asm/imx-common/iomux-v3.h > +++ b/arch/arm/include/asm/imx-common/iomux-v3.h > @@ -174,5 +174,7 @@ typedef u64 iomux_v3_cfg_t; > void imx_iomux_v3_setup_pad(iomux_v3_cfg_t pad); > void imx_iomux_v3_setup_multiple_pads(iomux_v3_cfg_t const *pad_list, > unsigned count); > +void mxc_iomux_set_gpr_register(int group, int start_bit, int num_bits, > + int value); > > #endif /* __MACH_IOMUX_V3_H__*/ > Best regards, Stefano Babic
Hello Stefano, Am 11.07.2014 13:10, schrieb Stefano Babic: > Hi Heiko, > > On 28/05/2014 12:16, Heiko Schocher wrote: >> add mxc_iomux_set_gpr_register fucntion >> >> Signed-off-by: Heiko Schocher<hs@denx.de> >> Cc: Stefano Babic<sbabic@denx.de> >> --- >> arch/arm/imx-common/iomux-v3.c | 15 +++++++++++++++ >> arch/arm/include/asm/imx-common/iomux-v3.h | 2 ++ >> 2 files changed, 17 insertions(+) >> >> diff --git a/arch/arm/imx-common/iomux-v3.c b/arch/arm/imx-common/iomux-v3.c >> index 6e46ea8..8739b4c 100644 >> --- a/arch/arm/imx-common/iomux-v3.c >> +++ b/arch/arm/imx-common/iomux-v3.c >> @@ -63,3 +63,18 @@ void imx_iomux_v3_setup_multiple_pads(iomux_v3_cfg_t const *pad_list, >> for (i = 0; i< count; i++) >> imx_iomux_v3_setup_pad(*p++); >> } >> + >> +void mxc_iomux_set_gpr_register(int group, int start_bit, int num_bits, >> + int value) >> +{ >> + int i = 0; >> + u32 reg; >> + reg = __raw_readl(base + group * 4); >> + while (num_bits) { >> + reg&= ~(1<<(start_bit + i)); >> + i++; >> + num_bits--; >> + } >> + reg |= (value<< start_bit); >> + __raw_writel(reg, base + group * 4); >> +} > > It is not clear to me why we need to add this or better, I want to avoid > having twice quite the same code doing the same ;-). > > We have already a field for gpr: > struct iomuxc { > u32 gpr[14]; > u32 omux[5]; > /* mux and pad registers */ > }; Ah! > + reg = __raw_readl(base + group * 4); > > this could not work when Freescale introduce a new variant with a slight > different layout. Thanks to Fabio, we know this is already reality: > > http://patchwork.ozlabs.org/patch/368336/ > > Your code *does* not work with the solox. > > Generally, we access register and we use setbits_ and clrsetbits_ to > modify single bits. What is the advantage here ? None, patch not longer needed. I remove it from the next patchserie. Thanks! bye, Heiko
diff --git a/arch/arm/imx-common/iomux-v3.c b/arch/arm/imx-common/iomux-v3.c index 6e46ea8..8739b4c 100644 --- a/arch/arm/imx-common/iomux-v3.c +++ b/arch/arm/imx-common/iomux-v3.c @@ -63,3 +63,18 @@ void imx_iomux_v3_setup_multiple_pads(iomux_v3_cfg_t const *pad_list, for (i = 0; i < count; i++) imx_iomux_v3_setup_pad(*p++); } + +void mxc_iomux_set_gpr_register(int group, int start_bit, int num_bits, + int value) +{ + int i = 0; + u32 reg; + reg = __raw_readl(base + group * 4); + while (num_bits) { + reg &= ~(1<<(start_bit + i)); + i++; + num_bits--; + } + reg |= (value << start_bit); + __raw_writel(reg, base + group * 4); +} diff --git a/arch/arm/include/asm/imx-common/iomux-v3.h b/arch/arm/include/asm/imx-common/iomux-v3.h index cca920b..4c83617 100644 --- a/arch/arm/include/asm/imx-common/iomux-v3.h +++ b/arch/arm/include/asm/imx-common/iomux-v3.h @@ -174,5 +174,7 @@ typedef u64 iomux_v3_cfg_t; void imx_iomux_v3_setup_pad(iomux_v3_cfg_t pad); void imx_iomux_v3_setup_multiple_pads(iomux_v3_cfg_t const *pad_list, unsigned count); +void mxc_iomux_set_gpr_register(int group, int start_bit, int num_bits, + int value); #endif /* __MACH_IOMUX_V3_H__*/
add mxc_iomux_set_gpr_register fucntion Signed-off-by: Heiko Schocher <hs@denx.de> Cc: Stefano Babic <sbabic@denx.de> --- arch/arm/imx-common/iomux-v3.c | 15 +++++++++++++++ arch/arm/include/asm/imx-common/iomux-v3.h | 2 ++ 2 files changed, 17 insertions(+)