[3/4] pinctrl: sh-pfc: Use u32 to store register data

As PFC registers are either 8, 16, or 32 bits wide, use u32 (mostly
replacing unsigned long) to store (parts of) register values and masks.

Switch the shadow register operations from {set,clear}_bit() to plain C
bit operations, as the former can operate on long data only.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pinctrl/sh-pfc/core.c | 25 +++++++++++++------------
 drivers/pinctrl/sh-pfc/core.h |  5 ++---
 drivers/pinctrl/sh-pfc/gpio.c | 13 ++++++-------
 3 files changed, 21 insertions(+), 22 deletions(-)

Hi Geert,

Thank you for the patch.

On Friday 27 February 2015 18:38:04 Geert Uytterhoeven wrote:
> As PFC registers are either 8, 16, or 32 bits wide, use u32 (mostly
> replacing unsigned long) to store (parts of) register values and masks.

I hope we won't get 64-bit registers in the future... Fingers crossed.

> Switch the shadow register operations from {set,clear}_bit() to plain C
> bit operations, as the former can operate on long data only.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/pinctrl/sh-pfc/core.c | 25 +++++++++++++------------
>  drivers/pinctrl/sh-pfc/core.h |  5 ++---
>  drivers/pinctrl/sh-pfc/gpio.c | 13 ++++++-------
>  3 files changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
> index 466b899ec78b15d7..1758043cfcec253b 100644
> --- a/drivers/pinctrl/sh-pfc/core.c
> +++ b/drivers/pinctrl/sh-pfc/core.c
> @@ -144,8 +144,7 @@ static int sh_pfc_enum_in_range(u16 enum_id, const
> struct pinmux_range *r) return 1;
>  }
> 
> -unsigned long sh_pfc_read_raw_reg(void __iomem *mapped_reg,
> -				  unsigned long reg_width)
> +u32 sh_pfc_read_raw_reg(void __iomem *mapped_reg, unsigned long reg_width)
>  {
>  	switch (reg_width) {
>  	case 8:
> @@ -161,7 +160,7 @@ unsigned long sh_pfc_read_raw_reg(void __iomem
> *mapped_reg, }
> 
>  void sh_pfc_write_raw_reg(void __iomem *mapped_reg, unsigned long
> reg_width,
> -			  unsigned long data)
> +			  u32 data)
>  {
>  	switch (reg_width) {
>  	case 8:
> @@ -181,8 +180,7 @@ void sh_pfc_write_raw_reg(void __iomem *mapped_reg,
> unsigned long reg_width, static void sh_pfc_config_reg_helper(struct sh_pfc
> *pfc,
>  				     const struct pinmux_cfg_reg *crp,
>  				     unsigned long in_pos,
> -				     void __iomem **mapped_regp,
> -				     unsigned long *maskp,
> +				     void __iomem **mapped_regp, u32 *maskp,
>  				     unsigned long *posp)
>  {
>  	unsigned int k;
> @@ -202,14 +200,15 @@ static void sh_pfc_config_reg_helper(struct sh_pfc
> *pfc,
> 
>  static void sh_pfc_write_config_reg(struct sh_pfc *pfc,
>  				    const struct pinmux_cfg_reg *crp,
> -				    unsigned long field, unsigned long value)
> +				    unsigned long field, u32 value)
>  {
>  	void __iomem *mapped_reg;
> -	unsigned long mask, pos, data;
> +	unsigned long pos;
> +	u32 mask, data;
> 
>  	sh_pfc_config_reg_helper(pfc, crp, field, &mapped_reg, &mask, &pos);
> 
> -	dev_dbg(pfc->dev, "write_reg addr = %lx, value = %ld, field = %ld, "
> +	dev_dbg(pfc->dev, "write_reg addr = %lx, value = 0x%x, field = %ld, "
>  		"r_width = %u, f_width = %u\n",
>  		crp->reg, value, field, crp->reg_width, crp->field_width);
> 
> @@ -230,11 +229,12 @@ static void sh_pfc_write_config_reg(struct sh_pfc
> *pfc,
> 
>  static int sh_pfc_get_config_reg(struct sh_pfc *pfc, u16 enum_id,
>  				 const struct pinmux_cfg_reg **crp, int *fieldp,
> -				 int *valuep)
> +				 u32 *valuep)
>  {
>  	const struct pinmux_cfg_reg *config_reg;
> -	unsigned long r_width, f_width, curr_width, ncomb;
> -	unsigned int k, m, n, pos, bit_pos;
> +	unsigned long r_width, f_width, curr_width;
> +	unsigned int k, m, pos, bit_pos;
> +	u32 ncomb, n;

Nitpicking, strictly speaking ncomb and n are not register data, but using u32 
for them seems good to me.

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	k = 0;
>  	while (1) {
> @@ -300,7 +300,8 @@ int sh_pfc_config_mux(struct sh_pfc *pfc, unsigned mark,
> int pinmux_type) const struct pinmux_cfg_reg *cr = NULL;
>  	u16 enum_id;
>  	const struct pinmux_range *range;
> -	int in_range, pos, field, value;
> +	int in_range, pos, field;
> +	u32 value;
>  	int ret;
> 
>  	switch (pinmux_type) {
> diff --git a/drivers/pinctrl/sh-pfc/core.h b/drivers/pinctrl/sh-pfc/core.h
> index 6b59d63b9c01e7a6..8a10dd50ccdd2e0c 100644
> --- a/drivers/pinctrl/sh-pfc/core.h
> +++ b/drivers/pinctrl/sh-pfc/core.h
> @@ -57,10 +57,9 @@ int sh_pfc_unregister_gpiochip(struct sh_pfc *pfc);
>  int sh_pfc_register_pinctrl(struct sh_pfc *pfc);
>  int sh_pfc_unregister_pinctrl(struct sh_pfc *pfc);
> 
> -unsigned long sh_pfc_read_raw_reg(void __iomem *mapped_reg,
> -				  unsigned long reg_width);
> +u32 sh_pfc_read_raw_reg(void __iomem *mapped_reg, unsigned long reg_width);
> void sh_pfc_write_raw_reg(void __iomem *mapped_reg, unsigned long
> reg_width, -			  unsigned long data);
> +			  u32 data);
> 
>  int sh_pfc_get_pin_index(struct sh_pfc *pfc, unsigned int pin);
>  int sh_pfc_config_mux(struct sh_pfc *pfc, unsigned mark, int pinmux_type);
> diff --git a/drivers/pinctrl/sh-pfc/gpio.c b/drivers/pinctrl/sh-pfc/gpio.c
> index 80f641ee4dea3146..f2bb7d7398cdfc24 100644
> --- a/drivers/pinctrl/sh-pfc/gpio.c
> +++ b/drivers/pinctrl/sh-pfc/gpio.c
> @@ -21,7 +21,7 @@
> 
>  struct sh_pfc_gpio_data_reg {
>  	const struct pinmux_data_reg *info;
> -	unsigned long shadow;
> +	u32 shadow;
>  };
> 
>  struct sh_pfc_gpio_pin {
> @@ -59,8 +59,8 @@ static void gpio_get_data_reg(struct sh_pfc_chip *chip,
> unsigned int offset, *bit = gpio_pin->dbit;
>  }
> 
> -static unsigned long gpio_read_data_reg(struct sh_pfc_chip *chip,
> -					const struct pinmux_data_reg *dreg)
> +static u32 gpio_read_data_reg(struct sh_pfc_chip *chip,
> +			      const struct pinmux_data_reg *dreg)
>  {
>  	void __iomem *mem = dreg->reg - chip->mem->phys + chip->mem->virt;
> 
> @@ -68,8 +68,7 @@ static unsigned long gpio_read_data_reg(struct sh_pfc_chip
> *chip, }
> 
>  static void gpio_write_data_reg(struct sh_pfc_chip *chip,
> -				const struct pinmux_data_reg *dreg,
> -				unsigned long value)
> +				const struct pinmux_data_reg *dreg, u32 value)
>  {
>  	void __iomem *mem = dreg->reg - chip->mem->phys + chip->mem->virt;
> 
> @@ -162,9 +161,9 @@ static void gpio_pin_set_value(struct sh_pfc_chip *chip,
> unsigned offset, pos = reg->info->reg_width - (bit + 1);
> 
>  	if (value)
> -		set_bit(pos, &reg->shadow);
> +		reg->shadow |= BIT(pos);
>  	else
> -		clear_bit(pos, &reg->shadow);
> +		reg->shadow &= ~BIT(pos);
> 
>  	gpio_write_data_reg(chip, reg->info, reg->shadow);
>  }

Hi Laurent,

On Thu, Mar 5, 2015 at 10:14 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> @@ -230,11 +229,12 @@ static void sh_pfc_write_config_reg(struct sh_pfc
>> *pfc,
>>
>>  static int sh_pfc_get_config_reg(struct sh_pfc *pfc, u16 enum_id,
>>                                const struct pinmux_cfg_reg **crp, int *fieldp,
>> -                              int *valuep)
>> +                              u32 *valuep)
>>  {
>>       const struct pinmux_cfg_reg *config_reg;
>> -     unsigned long r_width, f_width, curr_width, ncomb;
>> -     unsigned int k, m, n, pos, bit_pos;
>> +     unsigned long r_width, f_width, curr_width;
>> +     unsigned int k, m, pos, bit_pos;
>> +     u32 ncomb, n;
>
> Nitpicking, strictly speaking ncomb and n are not register data, but using u32
> for them seems good to me.

n is assigned to the (partial) register value output parameter, so it is.
ncomb is an upper boundary for n.

And I didn't want to split this off to yet another patch ;-)

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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Fri, Feb 27, 2015 at 6:38 PM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:

> As PFC registers are either 8, 16, or 32 bits wide, use u32 (mostly
> replacing unsigned long) to store (parts of) register values and masks.
>
> Switch the shadow register operations from {set,clear}_bit() to plain C
> bit operations, as the former can operate on long data only.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Patch applied with Laurent's ACK.

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

Hi Linus,

On Mon, Mar 9, 2015 at 5:37 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Feb 27, 2015 at 6:38 PM, Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
>
>> As PFC registers are either 8, 16, or 32 bits wide, use u32 (mostly
>> replacing unsigned long) to store (parts of) register values and masks.
>>
>> Switch the shadow register operations from {set,clear}_bit() to plain C
>> bit operations, as the former can operate on long data only.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Patch applied with Laurent's ACK.

Thanks!

I will post the reworked/new patches when I see your published branch.

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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html