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

[Yu-Chun Lin [林祐君]]

> 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
> 

Thanks for agreeing. Just to clarify, returning both bits in the first xlate
actually wouldn't work functionally.

If 'mask' includes both the data and WREN masks, clearing the GPIO will
inadvertently clear the WREN bit in 'val' as well.

	ret = gpio->reg_mask_xlate(..., &mask);
	/* If output is 0, WREN bit in val becomes 0*/
	val = output ? mask : 0;

Since the WREN bit must always be '1' for the hardware to update, we must
append it after 'val' is determined.

Best Regards,
Yu-Chun