Re: [PATCH v3 3/7] gpio: regmap: Add gpio_regmap_operation and write-enable support

["Michael Walle" <mwalle@kernel.org>]

Hi,

>> int (*value_xlate)(struct gpio_chip *chip, enum gpio_regmap_op op, unsigned
>> int offset, unsigned int reg, unsigned int mask, unsigned int *value)
>> 
>> mask shouldn't need to be a pointer as you don't have to modify it, do you?
>> reg_mask_xlate, already does that.
>> 
>
> Actually, I do need to modify the 'mask', so it still needs to be passed
> as a pointer to this callback.
>
> Since the WREN enable bit and the data bit are located at different bit
> positions, the final 'mask' used in write API must be expanded to include
> both the WREN mask and the data mask.

You could still return both bits in the first xlate, no?

But yeah, that might be a bit asymmetrical and not intuitive. So
changing the mask (again) with the value sounds ok to me.

-michael

> Aside from needing 'mask' as a pointer, I completely agree with the rest of
> your suggested signature.
>
> Best Regards,
> Yu-Chun
>
>> -michael
>> 
>> >               if (ret)
>> >                       return ret;
>> >       }
>> >
>> >       /* ignore input values which shadow the old output value */
>> >       if (gpio->reg_dat_base == gpio->reg_set_base)
>> >               ret = regmap_write_bits(gpio->regmap, reg, mask,
>> mask_val);
>> >       else
>> >               ret = regmap_update_bits(gpio->regmap, reg, mask,
>> > mask_val);
>> >
>> >       return ret;
>> > }
>> >
>> > Best Regards,
>> > Yu-Chun