On 8/12/24 12:22, Andy Shevchenko wrote:
> On Tue, Jul 30, 2024 at 07:16:10PM +0530, Sai Kumar Cholleti wrote:
>
> Please, refer to the functions in the text as func(), e.g.
exar_set_value().
> Use proper acronym, i.e. GPIO (capitalised).
We will update the patch and send a new version out if the current
approach is acceptable.
>> Before the gpio direction is changed from input to output,
>> exar_set_value is set to 1, but since direction is input when
>> exar_set_value is called, _regmap_update_bits reads a 1 due to an
>> external pull-up. When force_write is not set (regmap_set_bits has
>> force_write = false), the value is not written. When the direction is
>> then changed, the gpio becomes an output with the value of 0 (the
>> hardware default).
>>
>> regmap_write_bits sets force_write = true, so the value is always
>> written by exar_set_value and an external pull-up doesn't affect the
>> outcome of setting direction = high.
> Okay, shouldn't you instead mark the respective registers as volatile
or so?
> I believe regmap has some settings for this case. But I haven't
checked myself.
Unfortunately, in addition to marking the regmap volatile, we'd need to
define reg_update_bits which means we'd be partially undoing the work
from 36fb7218e87833b17e3042e77f3b102c75129e8f to reuse regmap locking
and update functions.
Below is the relevant section of _regmap_update_bits().
static int _regmap_update_bits(struct regmap *map, unsigned int reg,
unsigned int mask, unsigned int val,
bool *change, bool force_write)
...
if (regmap_volatile(map, reg) && map->reg_update_bits) {
...
} else {
...
if (force_write || (tmp != orig) ||
map->force_write_field) {
ret = _regmap_write(map, reg, tmp);
if (ret == 0 && change)
*change = true;
...
I suspect this might be a common problem with other GPIO drivers as
well.
Thank you,
Matthew
